superreview denied: [Bug 309671] Support %-escaped hostnames per RFC 3986 (3.2.2) : [Attachment 200546] updated patch (again a combined version for bugs 309672 and bug 309671)

Darin Fisher <darin@meer.net> has denied Andreas Otte
<andreas.otte@debitel.net>'s request for superreview:
Bug 309671: Support %-escaped hostnames per RFC 3986 (3.2.2)
https://bugzilla.mozilla.org/show_bug.cgi?id=309671

Attachment 200546: updated patch (again a combined version for bugs 309672 and
bug 309671)
https://bugzilla.mozilla.org/attachment.cgi?id=200546&action=edit

------- Additional Comments from Darin Fisher <darin@meer.net>
>Index: mozilla/browser/locales/en-US/chrome/overrides/appstrings.properties

>+invalidHost=%S seems to be an invalid hostname.


>Index: mozilla/browser/locales/en-US/chrome/overrides/netError.dtd

>+<!ENTITY invalidHost.title "Hostname invalid">
>+<!ENTITY invalidHost.longDesc "
>+<p>The provided hostname is not in a recognized format. If you clicked on a 
>+link most likely someone tries to trick you, otherwise check the location 
>+bar for mistakes and try again.</p>
>+">

Grammatical nits:

  "If you clicked on a link, most likely someone is trying to trick you.
   Otherwise, check the location bar for mistakes, and try again."

Perhaps the version in appstrings.properties should match the first
sentence from netError.dtd?

Also, I think we should ask beltzner to review this text.


>Index: mozilla/docshell/base/nsDocShell.cpp

>	  NS_ENSURE_ARG_POINTER(aURI);
>	  // Get the host
>	  nsCAutoString host;
>	  aURI->GetHost(host);
>	  CopyUTF8toUTF16(host, formatStrs[0]);
>	  formatStrCount = 1;
>	  error.AssignLiteral("dnsNotFound");
>     }
>+    else if (NS_ERROR_INVALID_HOST == aError) {
>+	  NS_ENSURE_ARG_POINTER(aURI);
>+	  // Get the host
>+	  nsCAutoString host;
>+	  aURI->GetHost(host);
>+	  CopyUTF8toUTF16(host, formatStrs[0]);
>+	  formatStrCount = 1;
>+	  error.AssignLiteral("invalidHost");

The code in the above block is almost identical.  Perhaps 
you should find a way to combine these so they do not
duplicate code so much.


>Index: mozilla/netwerk/base/public/nsNetError.h

> /**
>+ * The hostname is invalid.
>+ */
>+
>+#define NS_ERROR_INVALID_HOST \
>+    NS_ERROR_GENERATE_FAILURE(NS_ERROR_MODULE_NETWORK, 26)

"invalid" is such a vague description for this error code.  A hostname
that cannot be found via DNS could surely be termed invalid as well.
I think you need to specify more clearly what it means for a hostname
to be invalid.	In other words, I'm okay with the term "invalid" so
long as you provide ample documentation about what it means.  You might
want to say that it means that the hostname contains characters that
are considered invalid (i.e., to indicate that the hostname was rejected
before it was even sent to the DNS resolver).


>Index: mozilla/netwerk/base/src/nsStandardURL.cpp

> PRBool
> nsStandardURL::NormalizeIDN(const nsCSubstring &host, nsCString &result)
> {
>+    // First unescape the host to make it possibly UTF8, then check if
>+    // it is ASCII
>     // If host is ACE, then convert to UTF-8.  Else, if host is already
UTF-8,
>     // then make sure it is normalized per IDN.

Please make form the comments as a complete paragraph with a period between
sentences:

      // First unescape the host to make it possibly UTF-8, then check if
      // it is ASCII.  If host is ACE, then convert to UTF-8.  Else, if host
      // is already UTF-8, then make sure it is normalized per IDN.
     

>+    // unescape 
>+    nsCAutoString unescapedHost;
>+    NS_UnescapeURL(host, esc_AlwaysCopy, unescapedHost);
>+
>+    // now check if it really is ASCII
>+    if (IsASCII(unescapedHost)) {

For performance reasons, I think we should avoid copying the hostname like
this since in most cases the hostname will not be escaped.  I realize that
the methods in nsEscape.h are not very helpful in this regard, so this is
fine for now.


>Index: mozilla/netwerk/dns/src/nsDNSService2.cpp

>     const nsACString *hostPtr = &hostname;
> 
>+    // first unescape the hostname to catch encodings 
>+    nsCAutoString unescapedHost; 
>+    if (NS_UnescapeURL(PromiseFlatCString(hostname).get(), hostname.Length(),
 
>+			 0, unescapedHost)) {
>+	  hostPtr = &unescapedHost;
>+    }

So, it turns out that you can just use hostname.BeginReading()
instead of PromiseFlatCString.	bsmedberg just added a version
of BeginReading to nsACString that returns a |const char *|.
NOTE: the result of BeginReading may not be null terminated.


>+    // first unescape the hostname to catch encodings 
>+    nsCAutoString unescapedHost; 
>+    if (NS_UnescapeURL(PromiseFlatCString(hostname).get(), hostname.Length(),
 
>+			 0, unescapedHost)) {
>+	  hostPtr = &unescapedHost;
>+    }

ditto: use BeginReading instead of PromiseFlatCString.


>Index: mozilla/dom/locales/en-US/chrome/appstrings.properties

>+invalidHost=%S is invalid. Please check the hostname and try again.

>Index: mozilla/dom/locales/en-US/chrome/netError.dtd

>+<!ENTITY invalidHost.title "Hostname Invalid">
>+<!ENTITY invalidHost.longDesc "<p>The provided hostname is not in a
recognized format. If you clicked on a link most likely someone tries to trick
you, otherwise check the location bar for mistakes and try again.</p>">
>+

Again, these strings need to be reviewed by Mike Beltzner.
0
bugzilla
11/10/2005 12:31:18 AM
netscape.mozilla.reviewers 29156 articles. 0 followers. Follow

0 Replies
489 Views

Similar Articles

[PageSpeed] 19

Reply:

Similar Artilces:

superreview requested: [Bug 309671] Support %-escaped hostnames per RFC 3986 (3.2.2) : [Attachment 200546] updated patch (again a combined version for bugs 309672 and bug 309671)
Andreas Otte <andreas.otte@debitel.net> has asked Darin Fisher <darin@meer.net> for superreview: Bug 309671: Support %-escaped hostnames per RFC 3986 (3.2.2) https://bugzilla.mozilla.org/show_bug.cgi?id=309671 Attachment 200546: updated patch (again a combined version for bugs 309672 and bug 309671) https://bugzilla.mozilla.org/attachment.cgi?id=200546&action=edit ------- Additional Comments from Andreas Otte <andreas.otte@debitel.net> carrying over biesis review ...

superreview requested: [Bug 309671] Support %-escaped hostnames per RFC 3986 (3.2.2) : [Attachment 198453] merged patch for bug 309672 and 309671
Andreas Otte <andreas.otte@debitel.net> has asked Darin Fisher <darin@meer.net> for superreview: Bug 309671: Support %-escaped hostnames per RFC 3986 (3.2.2) https://bugzilla.mozilla.org/show_bug.cgi?id=309671 Attachment 198453: merged patch for bug 309672 and 309671 https://bugzilla.mozilla.org/attachment.cgi?id=198453&action=edit ------- Additional Comments from Andreas Otte <andreas.otte@debitel.net> requesting reviews but be aware that this patch is actually for bugs 309671 and 309672 ...

superreview cancelled: [Bug 309671] Support %-escaped hostnames per RFC 3986 (3.2.2) : [Attachment 198453] merged patch for bug 309672 and 309671
Andreas Otte <andreas.otte@debitel.net> has cancelled Andreas Otte <andreas.otte@debitel.net>'s request for superreview: Bug 309671: Support %-escaped hostnames per RFC 3986 (3.2.2) https://bugzilla.mozilla.org/show_bug.cgi?id=309671 Attachment 198453: merged patch for bug 309672 and 309671 https://bugzilla.mozilla.org/attachment.cgi?id=198453&action=edit ...

superreview cancelled: [Bug 309671] Support %-escaped hostnames per RFC 3986 (3.2.2) : [Attachment 199605] patch for the 1.8 branch, %-escape support only
Andreas Otte <andreas.otte@debitel.net> has cancelled Andreas Otte <andreas.otte@debitel.net>'s request for superreview: Bug 309671: Support %-escaped hostnames per RFC 3986 (3.2.2) https://bugzilla.mozilla.org/show_bug.cgi?id=309671 Attachment 199605: patch for the 1.8 branch, %-escape support only https://bugzilla.mozilla.org/attachment.cgi?id=199605&action=edit ...

superreview requested: [Bug 309671] Support %-escaped hostnames per RFC 3986 (3.2.2) : [Attachment 199605] patch for the 1.8 branch, %-escape support only
Andreas Otte <andreas.otte@debitel.net> has asked Darin Fisher <darin@meer.net> for superreview: Bug 309671: Support %-escaped hostnames per RFC 3986 (3.2.2) https://bugzilla.mozilla.org/show_bug.cgi?id=309671 Attachment 199605: patch for the 1.8 branch, %-escape support only https://bugzilla.mozilla.org/attachment.cgi?id=199605&action=edit ...

superreview requested: [Bug 309671] Support %-escaped hostnames per RFC 3986 (3.2.2) / Cannot open IDN from other applications(e.g., from Thunderbird) : [Attachment 202585] updated rest of the patch
Andreas Otte <andreas.otte@debitel.net> has asked Darin Fisher <darin@meer.net> for superreview: Bug 309671: Support %-escaped hostnames per RFC 3986 (3.2.2) / Cannot open IDN from other applications(e.g., from Thunderbird) https://bugzilla.mozilla.org/show_bug.cgi?id=309671 Attachment 202585: updated rest of the patch incorporating darins comments and Masayuki Nakanos work https://bugzilla.mozilla.org/attachment.cgi?id=202585&action=edit ...

superreview requested: [Bug 309671] Support %-escaped hostnames per RFC 3986 (3.2.2) / Cannot open IDN from other applications(e.g., from Thunderbird) : [Attachment 206804] new version of the patch,
Andreas Otte <andreas.otte@debitel.net> has asked Darin Fisher (on vacation 12/23 - 1/8) <darin@meer.net> for superreview: Bug 309671: Support %-escaped hostnames per RFC 3986 (3.2.2) / Cannot open IDN from other applications(e.g., from Thunderbird) https://bugzilla.mozilla.org/show_bug.cgi?id=309671 Attachment 206804: new version of the patch, changed unescaping https://bugzilla.mozilla.org/attachment.cgi?id=206804&action=edit ...

superreview cancelled: [Bug 309671] Support %-escaped hostnames per RFC 3986 (3.2.2) / Cannot open IDN from other applications(e.g., from Thunderbird) : [Attachment 202585] updated rest of the patch
Masayuki Nakano (Mozilla Japan) <masayuki@d-toybox.com> has cancelled Andreas Otte <andreas.otte@debitel.net>'s request for superreview: Bug 309671: Support %-escaped hostnames per RFC 3986 (3.2.2) / Cannot open IDN from other applications(e.g., from Thunderbird) https://bugzilla.mozilla.org/show_bug.cgi?id=309671 Attachment 202585: updated rest of the patch incorporating darins comments and Masayuki Nakanos work https://bugzilla.mozilla.org/attachment.cgi?id=202585&action=edit ------- Additional Comments from Masayuki Nakano (Mozilla Japan) <masayuki@d-toyb...

superreview denied: [Bug 309671] Support %-escaped hostnames per RFC 3986 (3.2.2) : [Attachment 200632] converting blacklist to whitelist
Darin Fisher <darin@meer.net> has denied Andreas Otte <andreas.otte@debitel.net>'s request for superreview: Bug 309671: Support %-escaped hostnames per RFC 3986 (3.2.2) https://bugzilla.mozilla.org/show_bug.cgi?id=309671 Attachment 200632: converting blacklist to whitelist https://bugzilla.mozilla.org/attachment.cgi?id=200632&action=edit ------- Additional Comments from Darin Fisher <darin@meer.net> >+ // if one of the chars in the host >+ // is not one of the following return false >+ return net_FindCharNotInSet(host.BeginReading(), en...

superreview granted: [Bug 309671] Support %-escaped hostnames per RFC 3986 (3.2.2) / Cannot open IDN from other applications(e.g., from Thunderbird) : [Attachment 202731] separated and updated string
neil@parkwaycc.co.uk <neil.parkwaycc.co.uk@bluebottle.com> has granted Andreas Otte <andreas.otte@debitel.net>'s request for superreview: Bug 309671: Support %-escaped hostnames per RFC 3986 (3.2.2) / Cannot open IDN from other applications(e.g., from Thunderbird) https://bugzilla.mozilla.org/show_bug.cgi?id=309671 Attachment 202731: separated and updated strings for the new error NS_ERROR_INVALID_HOST v1.2 https://bugzilla.mozilla.org/attachment.cgi?id=202731&action=edit ...

superreview requested: [Bug 309671] Support %-escaped hostnames per RFC 3986 (3.2.2) / Cannot open IDN from other applications(e.g., from Thunderbird) : [Attachment 202731] separated and updated stri
Andreas Otte <andreas.otte@debitel.net> has asked neil@parkwaycc.co.uk <neil.parkwaycc.co.uk@bluebottle.com> for superreview: Bug 309671: Support %-escaped hostnames per RFC 3986 (3.2.2) / Cannot open IDN from other applications(e.g., from Thunderbird) https://bugzilla.mozilla.org/show_bug.cgi?id=309671 Attachment 202731: separated and updated strings for the new error NS_ERROR_INVALID_HOST v1.2 https://bugzilla.mozilla.org/attachment.cgi?id=202731&action=edit ------- Additional Comments from Andreas Otte <andreas.otte@debitel.net> The patches in this bug int...

superreview requested: [Bug 309671] Support %-escaped hostnames per RFC 3986 (3.2.2) : [Attachment 200632] converting blacklist to whitelist
Andreas Otte <andreas.otte@debitel.net> has asked Darin Fisher <darin@meer.net> for superreview: Bug 309671: Support %-escaped hostnames per RFC 3986 (3.2.2) https://bugzilla.mozilla.org/show_bug.cgi?id=309671 Attachment 200632: converting blacklist to whitelist https://bugzilla.mozilla.org/attachment.cgi?id=200632&action=edit ...

superreview denied: [Bug 309671] Support %-escaped hostnames per RFC 3986 ( 3.2.2) / Cannot open IDN from other applications(e.g., from Thunderbird)
Christian :Biesinger (Mozilla Corp.) <cbiesinger@gmx.at> has denied Benjamin Smedberg [:bs] (bsmedberg) Away Until 27-Aug <benjamin@smedbergs.us>'s request for superreview: Bug 309671: Support %-escaped hostnames per RFC 3986 (3.2.2) / Cannot open IDN from other applications(e.g., from Thunderbird) https://bugzilla.mozilla.org/show_bug.cgi?id=309671 Attachment 206804: new version of the patch, changed unescaping https://bugzilla.mozilla.org/attachment.cgi?id=206804&action=edit ------- Additional Comments from Christian :Biesinger (Mozilla Corp.) <cbiesinger...

superreview requested: [Bug 309671] Support %-escaped hostnames per RFC 3986 (3.2.2) / Cannot open IDN from other applications(e.g., from Thunderbird)
Benjamin Smedberg [:bs] (bsmedberg@) <benjamin@smedbergs.us> has asked Christian Biesinger (:bi) (CA) (Mozilla Corp.) <cbiesinger@gmx.at> for superreview: Bug 309671: Support %-escaped hostnames per RFC 3986 (3.2.2) / Cannot open IDN from other applications(e.g., from Thunderbird) https://bugzilla.mozilla.org/show_bug.cgi?id=309671 Attachment 206804: new version of the patch, changed unescaping https://bugzilla.mozilla.org/attachment.cgi?id=206804&action=edit ...

Web resources about - superreview denied: [Bug 309671] Support %-escaped hostnames per RFC 3986 (3.2.2) : [Attachment 200546] updated patch (again a combined version for bugs 309672 and bug 309671) - netscape.mozilla.reviewers

Biz & Finance Magazines
Australian magazine subscriptions price comparison.

Ehsan Akhgari
Bugzilla is an essential tool to the working process of Mozilla, and many of us spend a good portion of their day in Bugzilla. The Bugzilla Tweaks ...

Resources last updated: 1/3/2016 10:18:10 PM