* [PATCH 0/2] gitweb: remove invalid http-equiv="content-type"
@ 2022-03-07 3:37 Jason Yundt
2022-03-07 3:37 ` [PATCH 1/2] comment: fix typo Jason Yundt
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: Jason Yundt @ 2022-03-07 3:37 UTC (permalink / raw)
To: git; +Cc: Jeff King, Jason Yundt
See the second commit's message for more details.
Jason Yundt (2):
comment: fix typo
gitweb: remove invalid http-equiv="content-type"
gitweb/gitweb.perl | 4 +---
t/t9502-gitweb-standalone-parse-output.sh | 15 ++++++++++++++-
2 files changed, 15 insertions(+), 4 deletions(-)
--
2.35.1
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 1/2] comment: fix typo 2022-03-07 3:37 [PATCH 0/2] gitweb: remove invalid http-equiv="content-type" Jason Yundt @ 2022-03-07 3:37 ` Jason Yundt 2022-03-07 3:37 ` [PATCH 2/2] gitweb: remove invalid http-equiv="content-type" Jason Yundt ` (3 subsequent siblings) 4 siblings, 0 replies; 17+ messages in thread From: Jason Yundt @ 2022-03-07 3:37 UTC (permalink / raw) To: git; +Cc: Jeff King, Jason Yundt Signed-off-by: Jason Yundt <jason@jasonyundt.email> --- t/t9502-gitweb-standalone-parse-output.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh index 3167473b30..e7363511dd 100755 --- a/t/t9502-gitweb-standalone-parse-output.sh +++ b/t/t9502-gitweb-standalone-parse-output.sh @@ -34,7 +34,7 @@ EOF # # This will check that gitweb HTTP header contains proposed filename # as <basename> with '.tar' suffix added, and that generated tarfile -# (gitweb message body) has <prefix> as prefix for al files in tarfile +# (gitweb message body) has <prefix> as prefix for all files in tarfile # # <prefix> default to <basename> check_snapshot () { -- 2.35.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/2] gitweb: remove invalid http-equiv="content-type" 2022-03-07 3:37 [PATCH 0/2] gitweb: remove invalid http-equiv="content-type" Jason Yundt 2022-03-07 3:37 ` [PATCH 1/2] comment: fix typo Jason Yundt @ 2022-03-07 3:37 ` Jason Yundt 2022-03-07 12:23 ` Ævar Arnfjörð Bjarmason 2022-03-07 23:24 ` brian m. carlson 2022-03-08 1:07 ` [PATCH v2 0/2] " Jason Yundt ` (2 subsequent siblings) 4 siblings, 2 replies; 17+ messages in thread From: Jason Yundt @ 2022-03-07 3:37 UTC (permalink / raw) To: git; +Cc: Jeff King, Jason Yundt Before this change, gitweb would generate pages which included: <meta http-equiv="content-type" content="application/xhtml+xml; charset=utf-8"/> A meta element with http-equiv="content-type" is said to be in the "Encoding declaration state". According to the HTML Standard, The Encoding declaration state may be used in HTML documents, but elements with an http-equiv attribute in that state must not be used in XML documents. Source: <https://html.spec.whatwg.org/multipage/semantics.html#attr-meta-http-equiv-content-type> This change removes that meta element since gitweb always generates XML documents. Signed-off-by: Jason Yundt <jason@jasonyundt.email> --- gitweb/gitweb.perl | 4 +--- t/t9502-gitweb-standalone-parse-output.sh | 13 +++++++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index fbd1c20a23..606b50104c 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -4213,8 +4213,7 @@ sub git_header_html { my %opts = @_; my $title = get_page_title(); - my $content_type = get_content_type_html(); - print $cgi->header(-type=>$content_type, -charset => 'utf-8', + print $cgi->header(-type=>get_content_type_html(), -charset => 'utf-8', -status=> $status, -expires => $expires) unless ($opts{'-no_http_header'}); my $mod_perl_version = $ENV{'MOD_PERL'} ? " $ENV{'MOD_PERL'}" : ''; @@ -4225,7 +4224,6 @@ sub git_header_html { <!-- git web interface version $version, (C) 2005-2006, Kay Sievers <kay.sievers\@vrfy.org>, Christian Gierke --> <!-- git core binaries version $git_version --> <head> -<meta http-equiv="content-type" content="$content_type; charset=utf-8"/> <meta name="generator" content="gitweb/$version git/$git_version$mod_perl_version"/> <meta name="robots" content="index, nofollow"/> <title>$title</title> diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh index e7363511dd..25165edacc 100755 --- a/t/t9502-gitweb-standalone-parse-output.sh +++ b/t/t9502-gitweb-standalone-parse-output.sh @@ -207,4 +207,17 @@ test_expect_success 'xss checks' ' xss "" "$TAG+" ' +no_http_equiv_content_type() { + gitweb_run "$@" && + ! grep -Ei "http-equiv=['\"]?content-type" gitweb.body +} + +# See: <https://html.spec.whatwg.org/dev/semantics.html#attr-meta-http-equiv-content-type> +test_expect_success 'no http-equiv="content-type" in XHTML' ' + no_http_equiv_content_type && + no_http_equiv_content_type "p=.git" && + no_http_equiv_content_type "p=.git;a=log" && + no_http_equiv_content_type "p=.git;a=tree" +' + test_done -- 2.35.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] gitweb: remove invalid http-equiv="content-type" 2022-03-07 3:37 ` [PATCH 2/2] gitweb: remove invalid http-equiv="content-type" Jason Yundt @ 2022-03-07 12:23 ` Ævar Arnfjörð Bjarmason 2022-03-07 22:49 ` Jason Yundt 2022-03-07 23:24 ` brian m. carlson 1 sibling, 1 reply; 17+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2022-03-07 12:23 UTC (permalink / raw) To: Jason Yundt; +Cc: git, Jeff King On Sun, Mar 06 2022, Jason Yundt wrote: > Before this change, gitweb would generate pages which included: > > <meta http-equiv="content-type" content="application/xhtml+xml; charset=utf-8"/> > > A meta element with http-equiv="content-type" is said to be in the > "Encoding declaration state". According to the HTML Standard, > > The Encoding declaration state may be used in HTML documents, > but elements with an http-equiv attribute in that state must not > be used in XML documents. > > Source: <https://html.spec.whatwg.org/multipage/semantics.html#attr-meta-http-equiv-content-type> > > This change removes that meta element since gitweb always generates XML > documents. > > Signed-off-by: Jason Yundt <jason@jasonyundt.email> > --- > gitweb/gitweb.perl | 4 +--- > t/t9502-gitweb-standalone-parse-output.sh | 13 +++++++++++++ > 2 files changed, 14 insertions(+), 3 deletions(-) > > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl > index fbd1c20a23..606b50104c 100755 > --- a/gitweb/gitweb.perl > +++ b/gitweb/gitweb.perl > @@ -4213,8 +4213,7 @@ sub git_header_html { > my %opts = @_; > > my $title = get_page_title(); > - my $content_type = get_content_type_html(); > - print $cgi->header(-type=>$content_type, -charset => 'utf-8', > + print $cgi->header(-type=>get_content_type_html(), -charset => 'utf-8', I think it would be better to just skip this hunk, no behavior will change if it's left in. > -status=> $status, -expires => $expires) > unless ($opts{'-no_http_header'}); > my $mod_perl_version = $ENV{'MOD_PERL'} ? " $ENV{'MOD_PERL'}" : ''; > @@ -4225,7 +4224,6 @@ sub git_header_html { > <!-- git web interface version $version, (C) 2005-2006, Kay Sievers <kay.sievers\@vrfy.org>, Christian Gierke --> > <!-- git core binaries version $git_version --> > <head> > -<meta http-equiv="content-type" content="$content_type; charset=utf-8"/> ..with this being the only behavior change (yeah the variable will now be used only in one place, but that's fine) I'm not sure I understand this change really. The result in always XML, so application/xhtml+xml is redundant, text/html, or both? But aside from that: I have seen browsers get the lack of encoding="" "wrong" with data at rest, don't some still default to ISO-8859-1? So won't this result in badly decoded data if you save the web page & view it locally? > <meta name="generator" content="gitweb/$version git/$git_version$mod_perl_version"/> > <meta name="robots" content="index, nofollow"/> > <title>$title</title> > diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh > index e7363511dd..25165edacc 100755 > --- a/t/t9502-gitweb-standalone-parse-output.sh > +++ b/t/t9502-gitweb-standalone-parse-output.sh > @@ -207,4 +207,17 @@ test_expect_success 'xss checks' ' > xss "" "$TAG+" > ' > > +no_http_equiv_content_type() { > + gitweb_run "$@" && > + ! grep -Ei "http-equiv=['\"]?content-type" gitweb.body Nit: Should we skip the "-i" here since we're testing our own output, and not http standards in general (i.e. we don't have to worry about the case of http-equiv?) ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] gitweb: remove invalid http-equiv="content-type" 2022-03-07 12:23 ` Ævar Arnfjörð Bjarmason @ 2022-03-07 22:49 ` Jason Yundt 0 siblings, 0 replies; 17+ messages in thread From: Jason Yundt @ 2022-03-07 22:49 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git, Jeff King On Monday, March 7, 2022 7:23:49 AM EST Ævar Arnfjörð Bjarmason wrote: > I'm not sure I understand this change really. The result in always XML, > so application/xhtml+xml is redundant, text/html, or both? To be honest, using an http-equiv="content-type" in XHTML is confusing. When you do use one, your goal shouldn’t really be to specify the document’s MIME type. After all, the first three lines of each page say <?xml version="1.0" encoding="utf-8"?> <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"> <html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en-US" lang="en-US"> Those lines are more than enough to determine that something is using XHTML and UTF-8. Instead, the idea is to help out a parser that is incorrectly parsing the document as HTML (instead of as XHTML). Historical W3C documents (that were applicable when http-equiv="content-type" was allowed in XHTML) [1] [2][3] indicate that http-equiv="content-type" should be used like this: <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"/> In other words, to use http-equiv="content-type" properly in XHTML, you had to lie about the document’s type. The fact that this is confusing is probably part of why WHATWG disallowed it in the HTML Standard. > But aside from that: I have seen browsers get the lack of encoding="" > "wrong" with data at rest, don't some still default to ISO-8859-1? > > So won't this result in badly decoded data if you save the web page & > view it locally? I tested this idea in ungoogled-chromium, Firefox and Pale Moon. Other than Pale Moon in one specific circumstance, they all used UTF-8 as the encoding. Pale Moon used windows-1252, but only when the file ended with .html. When the file ended with .xhtml, Pale Moon used UTF-8. That being said, we don’t have to use an http-equiv="content-type" to fix the problem. Instead, we can use a <meta charset="utf-8"> which is allowed by the HTML Standard [4]. [1]: <https://www.w3.org/TR/xhtml1/#C_9> [2]: <https://www.w3.org/TR/html-polyglot/#character-encoding> [3]: <https://www.w3.org/Bugs/Public/show_bug.cgi?id=21818> [4]: <https://html.spec.whatwg.org/multipage/semantics.html#attr-meta-charset> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] gitweb: remove invalid http-equiv="content-type" 2022-03-07 3:37 ` [PATCH 2/2] gitweb: remove invalid http-equiv="content-type" Jason Yundt 2022-03-07 12:23 ` Ævar Arnfjörð Bjarmason @ 2022-03-07 23:24 ` brian m. carlson 1 sibling, 0 replies; 17+ messages in thread From: brian m. carlson @ 2022-03-07 23:24 UTC (permalink / raw) To: Jason Yundt; +Cc: git, Jeff King [-- Attachment #1: Type: text/plain, Size: 1772 bytes --] On 2022-03-07 at 03:37:23, Jason Yundt wrote: > Before this change, gitweb would generate pages which included: > > <meta http-equiv="content-type" content="application/xhtml+xml; charset=utf-8"/> > > A meta element with http-equiv="content-type" is said to be in the > "Encoding declaration state". According to the HTML Standard, > > The Encoding declaration state may be used in HTML documents, > but elements with an http-equiv attribute in that state must not > be used in XML documents. > > Source: <https://html.spec.whatwg.org/multipage/semantics.html#attr-meta-http-equiv-content-type> > > This change removes that meta element since gitweb always generates XML > documents. This change seems fine. We do specify this in the HTTP header, including the character set, which is what matters, so this should work in every browser, and the http-equiv is unneeded. I also don't think we need a meta header here, since we have an XML declaration, and that's controlling in this situation. This isn't regular HTML and we don't declare it as such, so using a meta header to control this isn't correct: the XML declaration should be used instead in the event a user downloads this to a local disk and processes it outside the context of an HTTP request. Since we control the HTTP headers, I'd actually argue that your test might well reject all http-equiv headers since they could be done much better with actual HTTP headers (and would therefore work with non-browser clients), but I don't think that's worth a reroll, nor do I think a test is even needed here (but bonus points for adding one). So I think this looks good as is. Thanks for the patch. -- brian m. carlson (he/him or they/them) Toronto, Ontario, CA [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 262 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 0/2] gitweb: remove invalid http-equiv="content-type" 2022-03-07 3:37 [PATCH 0/2] gitweb: remove invalid http-equiv="content-type" Jason Yundt 2022-03-07 3:37 ` [PATCH 1/2] comment: fix typo Jason Yundt 2022-03-07 3:37 ` [PATCH 2/2] gitweb: remove invalid http-equiv="content-type" Jason Yundt @ 2022-03-08 1:07 ` Jason Yundt 2022-03-08 2:13 ` Junio C Hamano 2022-03-08 15:56 ` [PATCH v3 " Jason Yundt 2022-03-08 1:07 ` [PATCH v2 1/2] comment: fix typo Jason Yundt 2022-03-08 1:07 ` [PATCH v2 2/2] gitweb: remove invalid http-equiv="content-type" Jason Yundt 4 siblings, 2 replies; 17+ messages in thread From: Jason Yundt @ 2022-03-08 1:07 UTC (permalink / raw) To: git; +Cc: Jason Yundt, Jeff King, Ævar Arnfjörð Bjarmason See the second commit's message for more details. Compared to the first version of this patch, this one - keeps an extra variable, - replaces the http-equiv="content-type" tag with a charset= one, and - removes the -i flag from grep. Jason Yundt (2): comment: fix typo gitweb: remove invalid http-equiv="content-type" gitweb/gitweb.perl | 2 +- t/t9502-gitweb-standalone-parse-output.sh | 18 +++++++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) -- 2.35.1 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 0/2] gitweb: remove invalid http-equiv="content-type" 2022-03-08 1:07 ` [PATCH v2 0/2] " Jason Yundt @ 2022-03-08 2:13 ` Junio C Hamano 2022-03-08 12:26 ` Jason Yundt 2022-03-08 15:56 ` [PATCH v3 " Jason Yundt 1 sibling, 1 reply; 17+ messages in thread From: Junio C Hamano @ 2022-03-08 2:13 UTC (permalink / raw) To: Jason Yundt; +Cc: git, Jeff King, Ævar Arnfjörð Bjarmason Jason Yundt <jason@jasonyundt.email> writes: > - keeps an extra variable, I am not sure if this is an improvement. The original had two places that used $content_type, but after getting rid of one, there is only one place that needed the value, which can be used in place; and it was quite clear that was what was going on in the previous iteration. About the <meta> thing, it seems that brian already commented on it. Thanks. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 0/2] gitweb: remove invalid http-equiv="content-type" 2022-03-08 2:13 ` Junio C Hamano @ 2022-03-08 12:26 ` Jason Yundt 0 siblings, 0 replies; 17+ messages in thread From: Jason Yundt @ 2022-03-08 12:26 UTC (permalink / raw) To: Junio C Hamano Cc: git, Jeff King, Ævar Arnfjörð Bjarmason, brian m. carlson On Monday, March 7, 2022 9:13:52 PM EST Junio C Hamano wrote: > About the <meta> thing, it seems that brian already commented on it. Thanks for mentioning that. I now see Brian’s comments on the archive. My mail server was blocking him (via zen.spamhaus.org), but I’ve added his server to the allowlist. ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 0/2] gitweb: remove invalid http-equiv="content-type" 2022-03-08 1:07 ` [PATCH v2 0/2] " Jason Yundt 2022-03-08 2:13 ` Junio C Hamano @ 2022-03-08 15:56 ` Jason Yundt 2022-03-08 15:56 ` [PATCH v3 1/2] comment: fix typo Jason Yundt 2022-03-08 15:56 ` [PATCH v3 2/2] gitweb: remove invalid http-equiv="content-type" Jason Yundt 1 sibling, 2 replies; 17+ messages in thread From: Jason Yundt @ 2022-03-08 15:56 UTC (permalink / raw) To: git Cc: Ævar Arnfjörð Bjarmason, brian m. carlson, Junio C Hamano, Jeff King, Jason Yundt See the second commit's message for more details. Compared to the second version of this patch, this one - removes the extra variable again, - doesn't include a <meta charset="utf-8"/> and - corrects a technical error in the second commit’s message. Jason Yundt (2): comment: fix typo gitweb: remove invalid http-equiv="content-type" gitweb/gitweb.perl | 4 +--- t/t9502-gitweb-standalone-parse-output.sh | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 4 deletions(-) -- 2.35.1 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 1/2] comment: fix typo 2022-03-08 15:56 ` [PATCH v3 " Jason Yundt @ 2022-03-08 15:56 ` Jason Yundt 2022-03-08 15:56 ` [PATCH v3 2/2] gitweb: remove invalid http-equiv="content-type" Jason Yundt 1 sibling, 0 replies; 17+ messages in thread From: Jason Yundt @ 2022-03-08 15:56 UTC (permalink / raw) To: git Cc: Ævar Arnfjörð Bjarmason, brian m. carlson, Junio C Hamano, Jeff King, Jason Yundt Signed-off-by: Jason Yundt <jason@jasonyundt.email> --- t/t9502-gitweb-standalone-parse-output.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh index 3167473b30..e7363511dd 100755 --- a/t/t9502-gitweb-standalone-parse-output.sh +++ b/t/t9502-gitweb-standalone-parse-output.sh @@ -34,7 +34,7 @@ EOF # # This will check that gitweb HTTP header contains proposed filename # as <basename> with '.tar' suffix added, and that generated tarfile -# (gitweb message body) has <prefix> as prefix for al files in tarfile +# (gitweb message body) has <prefix> as prefix for all files in tarfile # # <prefix> default to <basename> check_snapshot () { -- 2.35.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v3 2/2] gitweb: remove invalid http-equiv="content-type" 2022-03-08 15:56 ` [PATCH v3 " Jason Yundt 2022-03-08 15:56 ` [PATCH v3 1/2] comment: fix typo Jason Yundt @ 2022-03-08 15:56 ` Jason Yundt 1 sibling, 0 replies; 17+ messages in thread From: Jason Yundt @ 2022-03-08 15:56 UTC (permalink / raw) To: git Cc: Ævar Arnfjörð Bjarmason, brian m. carlson, Junio C Hamano, Jeff King, Jason Yundt Before this change, gitweb would generate pages which included: <meta http-equiv="content-type" content="application/xhtml+xml; charset=utf-8"/> When a meta's http-equiv equals "content-type", the http-equiv is said to be in the "Encoding declaration state". According to the HTML Standard, The Encoding declaration state may be used in HTML documents, but elements with an http-equiv attribute in that state must not be used in XML documents. Source: <https://html.spec.whatwg.org/multipage/semantics.html#attr-meta-http-equiv-content-type> This change removes that meta element since gitweb always generates XML documents. Signed-off-by: Jason Yundt <jason@jasonyundt.email> --- gitweb/gitweb.perl | 4 +--- t/t9502-gitweb-standalone-parse-output.sh | 13 +++++++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index fbd1c20a23..606b50104c 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -4213,8 +4213,7 @@ sub git_header_html { my %opts = @_; my $title = get_page_title(); - my $content_type = get_content_type_html(); - print $cgi->header(-type=>$content_type, -charset => 'utf-8', + print $cgi->header(-type=>get_content_type_html(), -charset => 'utf-8', -status=> $status, -expires => $expires) unless ($opts{'-no_http_header'}); my $mod_perl_version = $ENV{'MOD_PERL'} ? " $ENV{'MOD_PERL'}" : ''; @@ -4225,7 +4224,6 @@ sub git_header_html { <!-- git web interface version $version, (C) 2005-2006, Kay Sievers <kay.sievers\@vrfy.org>, Christian Gierke --> <!-- git core binaries version $git_version --> <head> -<meta http-equiv="content-type" content="$content_type; charset=utf-8"/> <meta name="generator" content="gitweb/$version git/$git_version$mod_perl_version"/> <meta name="robots" content="index, nofollow"/> <title>$title</title> diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh index e7363511dd..8cb582f0e6 100755 --- a/t/t9502-gitweb-standalone-parse-output.sh +++ b/t/t9502-gitweb-standalone-parse-output.sh @@ -207,4 +207,17 @@ test_expect_success 'xss checks' ' xss "" "$TAG+" ' +no_http_equiv_content_type() { + gitweb_run "$@" && + ! grep -E "http-equiv=['\"]?content-type" gitweb.body +} + +# See: <https://html.spec.whatwg.org/dev/semantics.html#attr-meta-http-equiv-content-type> +test_expect_success 'no http-equiv="content-type" in XHTML' ' + no_http_equiv_content_type && + no_http_equiv_content_type "p=.git" && + no_http_equiv_content_type "p=.git;a=log" && + no_http_equiv_content_type "p=.git;a=tree" +' + test_done -- 2.35.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 1/2] comment: fix typo 2022-03-07 3:37 [PATCH 0/2] gitweb: remove invalid http-equiv="content-type" Jason Yundt ` (2 preceding siblings ...) 2022-03-08 1:07 ` [PATCH v2 0/2] " Jason Yundt @ 2022-03-08 1:07 ` Jason Yundt 2022-03-08 1:07 ` [PATCH v2 2/2] gitweb: remove invalid http-equiv="content-type" Jason Yundt 4 siblings, 0 replies; 17+ messages in thread From: Jason Yundt @ 2022-03-08 1:07 UTC (permalink / raw) To: git; +Cc: Jason Yundt, Jeff King, Ævar Arnfjörð Bjarmason Signed-off-by: Jason Yundt <jason@jasonyundt.email> --- t/t9502-gitweb-standalone-parse-output.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh index 3167473b30..e7363511dd 100755 --- a/t/t9502-gitweb-standalone-parse-output.sh +++ b/t/t9502-gitweb-standalone-parse-output.sh @@ -34,7 +34,7 @@ EOF # # This will check that gitweb HTTP header contains proposed filename # as <basename> with '.tar' suffix added, and that generated tarfile -# (gitweb message body) has <prefix> as prefix for al files in tarfile +# (gitweb message body) has <prefix> as prefix for all files in tarfile # # <prefix> default to <basename> check_snapshot () { -- 2.35.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 2/2] gitweb: remove invalid http-equiv="content-type" 2022-03-07 3:37 [PATCH 0/2] gitweb: remove invalid http-equiv="content-type" Jason Yundt ` (3 preceding siblings ...) 2022-03-08 1:07 ` [PATCH v2 1/2] comment: fix typo Jason Yundt @ 2022-03-08 1:07 ` Jason Yundt 2022-03-08 1:50 ` brian m. carlson 4 siblings, 1 reply; 17+ messages in thread From: Jason Yundt @ 2022-03-08 1:07 UTC (permalink / raw) To: git; +Cc: Jason Yundt, Jeff King, Ævar Arnfjörð Bjarmason Before this change, gitweb would generate pages which included: <meta http-equiv="content-type" content="application/xhtml+xml; charset=utf-8"/> A meta element with http-equiv="content-type" is said to be in the "Encoding declaration state". According to the HTML Standard, The Encoding declaration state may be used in HTML documents, but elements with an http-equiv attribute in that state must not be used in XML documents. Source: <https://html.spec.whatwg.org/multipage/semantics.html#attr-meta-http-equiv-content-type> Gitweb always generates XML documents, so its use of http-equiv="content-type" was invalid. This change replaces that tag with <meta charset="utf-8"/> which is equivalent [1] and allowed in XML documents [2]. [1]: <https://developer.mozilla.org/en-US/docs/Web/HTML/Element/meta#attr-http-equiv> [2]: <https://html.spec.whatwg.org/multipage/semantics.html#attr-meta-charset> Signed-off-by: Jason Yundt <jason@jasonyundt.email> --- gitweb/gitweb.perl | 2 +- t/t9502-gitweb-standalone-parse-output.sh | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index fbd1c20a23..59457c1004 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -4225,7 +4225,7 @@ sub git_header_html { <!-- git web interface version $version, (C) 2005-2006, Kay Sievers <kay.sievers\@vrfy.org>, Christian Gierke --> <!-- git core binaries version $git_version --> <head> -<meta http-equiv="content-type" content="$content_type; charset=utf-8"/> +<meta charset="utf-8"/> <meta name="generator" content="gitweb/$version git/$git_version$mod_perl_version"/> <meta name="robots" content="index, nofollow"/> <title>$title</title> diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh index e7363511dd..0b06e2d6b0 100755 --- a/t/t9502-gitweb-standalone-parse-output.sh +++ b/t/t9502-gitweb-standalone-parse-output.sh @@ -207,4 +207,20 @@ test_expect_success 'xss checks' ' xss "" "$TAG+" ' +check_encoding_meta_element() { + gitweb_run "$@" && + ! grep -E "http-equiv=['\"]?content-type" gitweb.body && + grep -F '<meta charset="utf-8"/>' gitweb.body +} + +# One of those can be used in XHTML, the other one can't. See: +# <https://html.spec.whatwg.org/dev/semantics.html#attr-meta-charset> +# <https://html.spec.whatwg.org/dev/semantics.html#attr-meta-http-equiv-content-type> +test_expect_success 'no http-equiv="content-type", yes charset="utf-8"' ' + check_encoding_meta_element && + check_encoding_meta_element "p=.git" && + check_encoding_meta_element "p=.git;a=log" && + check_encoding_meta_element "p=.git;a=tree" +' + test_done -- 2.35.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] gitweb: remove invalid http-equiv="content-type" 2022-03-08 1:07 ` [PATCH v2 2/2] gitweb: remove invalid http-equiv="content-type" Jason Yundt @ 2022-03-08 1:50 ` brian m. carlson 2022-03-08 12:44 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 17+ messages in thread From: brian m. carlson @ 2022-03-08 1:50 UTC (permalink / raw) To: Jason Yundt; +Cc: git, Jeff King, Ævar Arnfjörð Bjarmason [-- Attachment #1: Type: text/plain, Size: 814 bytes --] On 2022-03-08 at 01:07:11, Jason Yundt wrote: > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl > index fbd1c20a23..59457c1004 100755 > --- a/gitweb/gitweb.perl > +++ b/gitweb/gitweb.perl > @@ -4225,7 +4225,7 @@ sub git_header_html { > <!-- git web interface version $version, (C) 2005-2006, Kay Sievers <kay.sievers\@vrfy.org>, Christian Gierke --> > <!-- git core binaries version $git_version --> > <head> > -<meta http-equiv="content-type" content="$content_type; charset=utf-8"/> > +<meta charset="utf-8"/> I don't actually think this is an improvement. I don't think it's necessary, considering we have an XML declaration and the HTTP header, both of which already say it's UTF-8 and will take precedence over this. -- brian m. carlson (he/him or they/them) Toronto, Ontario, CA [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 262 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] gitweb: remove invalid http-equiv="content-type" 2022-03-08 1:50 ` brian m. carlson @ 2022-03-08 12:44 ` Ævar Arnfjörð Bjarmason 2022-03-08 14:54 ` Jason Yundt 0 siblings, 1 reply; 17+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2022-03-08 12:44 UTC (permalink / raw) To: brian m. carlson; +Cc: Jason Yundt, git, Jeff King On Tue, Mar 08 2022, brian m. carlson wrote: > [[PGP Signed Part:Undecided]] > On 2022-03-08 at 01:07:11, Jason Yundt wrote: >> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl >> index fbd1c20a23..59457c1004 100755 >> --- a/gitweb/gitweb.perl >> +++ b/gitweb/gitweb.perl >> @@ -4225,7 +4225,7 @@ sub git_header_html { >> <!-- git web interface version $version, (C) 2005-2006, Kay Sievers <kay.sievers\@vrfy.org>, Christian Gierke --> >> <!-- git core binaries version $git_version --> >> <head> >> -<meta http-equiv="content-type" content="$content_type; charset=utf-8"/> >> +<meta charset="utf-8"/> > > I don't actually think this is an improvement. I don't think it's > necessary, considering we have an XML declaration and the HTTP header, > both of which already say it's UTF-8 and will take precedence over this. Ageed. I was a bit surprised per Jason's https://lore.kernel.org/git/109813056.nniJfEyVGO@jason-desktop-linux/ that the removal wasn't kept. I.e. he was replying to a question of mine asking whether we didn't need this data at rest, e.g if you save the page. I didn't notice the "<?xml version..." we emit, which seems to be enough. I.e. this seems to have always been redundant going back to c994d620cc8 (v220, 2005-08-07), or rather, the character set part of it. Maybe I still don't understand this, but the commit message seems to me be conflating whether we send the *right* http-equiv with whether we send it at all, i.e. if the problem is that XML documents shouldn't be text/html isn't this correct?: diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index fbd1c20a232..c1c5af0b197 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -4049,7 +4049,13 @@ sub get_page_title { return $title; } +sub get_content_type_xml { + return 'application/xhtml+xml'; +} + sub get_content_type_html { + my ($want_xml) = @_; + # require explicit support from the UA if we are to send the page as # 'application/xhtml+xml', otherwise send it as plain old 'text/html'. # we have to do this because MSIE sometimes globs '*/*', pretending to @@ -4057,7 +4063,7 @@ sub get_content_type_html { if (defined $cgi->http('HTTP_ACCEPT') && $cgi->http('HTTP_ACCEPT') =~ m/(,|;|\s|^)application\/xhtml\+xml(,|;|\s|$)/ && $cgi->Accept('application/xhtml+xml') != 0) { - return 'application/xhtml+xml'; + return get_content_type_html(); } else { return 'text/html'; } @@ -4214,6 +4220,7 @@ sub git_header_html { my $title = get_page_title(); my $content_type = get_content_type_html(); + my $content_type_xml = get_content_type_html(); print $cgi->header(-type=>$content_type, -charset => 'utf-8', -status=> $status, -expires => $expires) unless ($opts{'-no_http_header'}); @@ -4225,7 +4232,7 @@ sub git_header_html { <!-- git web interface version $version, (C) 2005-2006, Kay Sievers <kay.sievers\@vrfy.org>, Christian Gierke --> <!-- git core binaries version $git_version --> <head> -<meta http-equiv="content-type" content="$content_type; charset=utf-8"/> +<meta http-equiv="content-type" content="$content_type_xml; charset=utf-8"/> <meta name="generator" content="gitweb/$version git/$git_version$mod_perl_version"/> <meta name="robots" content="index, nofollow"/> <title>$title</title> Of course we might then *also* decide that <meta http-equiv> in this case isn't needed at all, but isn't that a seperate change? And won't conforming browsers treat application/xhtml+xml differently when the page is saved? A long time ago (Idid some web development) using it would enable pedantic strictness in browsers, i.e. unclosed tags etc. would be a hard error, but I can't reproduce that locally in either Firefox or Chrome now (with just the gitweb output as-is with that http-equiv tweaked). So maybe it does nothing, or maybe it's just those browser... ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] gitweb: remove invalid http-equiv="content-type" 2022-03-08 12:44 ` Ævar Arnfjörð Bjarmason @ 2022-03-08 14:54 ` Jason Yundt 0 siblings, 0 replies; 17+ messages in thread From: Jason Yundt @ 2022-03-08 14:54 UTC (permalink / raw) To: brian m. carlson, Ævar Arnfjörð Bjarmason; +Cc: git, Jeff King On Tuesday, March 8, 2022 7:44:35 AM EST Ævar Arnfjörð Bjarmason wrote: > Maybe I still don't understand this, but the commit message seems to me > be conflating whether we send the *right* http-equiv with whether we > send it at all, The intent behind the commit message is to say that <meta http-equiv="content-type" …> is never correct in XHTML. > i.e. if the problem is that XML documents shouldn't be > text/html isn't this correct?: > > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl > index fbd1c20a232..c1c5af0b197 100755 > --- a/gitweb/gitweb.perl > +++ b/gitweb/gitweb.perl > @@ -4049,7 +4049,13 @@ sub get_page_title { > return $title; > } > > +sub get_content_type_xml { > + return 'application/xhtml+xml'; > +} > + > sub get_content_type_html { > + my ($want_xml) = @_; > + > # require explicit support from the UA if we are to send the page as > # 'application/xhtml+xml', otherwise send it as plain old 'text/html'. > # we have to do this because MSIE sometimes globs '*/*', pretending to > @@ -4057,7 +4063,7 @@ sub get_content_type_html { > if (defined $cgi->http('HTTP_ACCEPT') && > $cgi->http('HTTP_ACCEPT') =~ m/(,|;|\s|^)application\/xhtml\+xml(,|;|\s|$)/ && > $cgi->Accept('application/xhtml+xml') != 0) { > - return 'application/xhtml+xml'; > + return get_content_type_html(); I’m guessing that you meant to call get_content_type_xml() here. > } else { > return 'text/html'; > } > @@ -4214,6 +4220,7 @@ sub git_header_html { > > my $title = get_page_title(); > my $content_type = get_content_type_html(); > + my $content_type_xml = get_content_type_html(); I’m also guessing that you meant to call get_content_type_xml() here. > print $cgi->header(-type=>$content_type, -charset => 'utf-8', > -status=> $status, -expires => $expires) > unless ($opts{'-no_http_header'}); > @@ -4225,7 +4232,7 @@ sub git_header_html { > <!-- git web interface version $version, (C) 2005-2006, Kay Sievers <kay.sievers\@vrfy.org>, Christian Gierke --> > <!-- git core binaries version $git_version --> > <head> > -<meta http-equiv="content-type" content="$content_type; charset=utf-8"/> > +<meta http-equiv="content-type" content="$content_type_xml; charset=utf-8"/> > <meta name="generator" content="gitweb/$version git/$git_version$mod_perl_version"/> > <meta name="robots" content="index, nofollow"/> > <title>$title</title> With those assumptions in mind, I don’t think that your code is correct if the problem is that XML documents shouldn't be text/html. Here’s why: 1. XML documents shouldn’t contain http-equiv="content-type" [1]. 2. When a meta’s http-equiv attribute equals content-type, then its content attribute should equal “the literal string "text/html;", optionally followed by any number of ASCII whitespace, followed by the literal string "charset=utf-8".” [1] [1]: <https://html.spec.whatwg.org/multipage/semantics.html#attr-meta-http-equiv-content-type> ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2022-03-08 15:59 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-03-07 3:37 [PATCH 0/2] gitweb: remove invalid http-equiv="content-type" Jason Yundt 2022-03-07 3:37 ` [PATCH 1/2] comment: fix typo Jason Yundt 2022-03-07 3:37 ` [PATCH 2/2] gitweb: remove invalid http-equiv="content-type" Jason Yundt 2022-03-07 12:23 ` Ævar Arnfjörð Bjarmason 2022-03-07 22:49 ` Jason Yundt 2022-03-07 23:24 ` brian m. carlson 2022-03-08 1:07 ` [PATCH v2 0/2] " Jason Yundt 2022-03-08 2:13 ` Junio C Hamano 2022-03-08 12:26 ` Jason Yundt 2022-03-08 15:56 ` [PATCH v3 " Jason Yundt 2022-03-08 15:56 ` [PATCH v3 1/2] comment: fix typo Jason Yundt 2022-03-08 15:56 ` [PATCH v3 2/2] gitweb: remove invalid http-equiv="content-type" Jason Yundt 2022-03-08 1:07 ` [PATCH v2 1/2] comment: fix typo Jason Yundt 2022-03-08 1:07 ` [PATCH v2 2/2] gitweb: remove invalid http-equiv="content-type" Jason Yundt 2022-03-08 1:50 ` brian m. carlson 2022-03-08 12:44 ` Ævar Arnfjörð Bjarmason 2022-03-08 14:54 ` Jason Yundt
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.