Bug 1635648 Comment 5 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to Kai Engert (:KaiE:) from comment #4)
> Who might know the answer to comment 3 ?

I throw in another question/comment here.

Many months (a few years) ago, I noticed strange errors dumped from SetSpec() call.
Some URIs that were passed to it were not acceptable, it complained.
But TB chugged along.
I believe I entered a bugzilla about it, but unfortunately,
somehow the error message dump code was removed. Basically it was hidden under the rug.
And I cannot find the bugzilla easily.
I doubt the wisdom of removing it.

Anyway, I have maintained the following local patch to dump the error of SetSpec() in one place for my local testing.
That logging is done very aptly in a file called nsIURIMutator.idl.
This URI mutation and what protocols are valid and what are not should be documented clearly for TB maintenance.

I used to log the SUCCESS case as well in my local dump because I could not figure out what "protocol" are accepted by TB (and presumably FF, but FF does not need to grok pop3, imap, mailbox, etc.).
Here is the patch to log only the failed case. There is a protection that protects the code from an arbitrarily long data. (That is why we see a large data or URI cut short in a log I am about to upload.) 
```
# HG changeset patch
# User ISHIKAWA, Chiaki <ishikawa@yk.rim.or.jp>
# Parent  f8e3af9ab87255463730e08a419b56b1fa1c1135
dump aSpec(i.e., path) for SetSpec failure

diff --git a/netwerk/base/nsIURIMutator.idl b/netwerk/base/nsIURIMutator.idl
--- a/netwerk/base/nsIURIMutator.idl
+++ b/netwerk/base/nsIURIMutator.idl
@@ -354,16 +354,29 @@ public:
   }
 
   NS_MutateURI& SetSpec(const nsACString& aSpec)
   {
     if (NS_FAILED(mStatus)) {
       return *this;
     }
     mStatus = mMutator->SetSpec(aSpec, nullptr);
+#if DEBUG
+    if (NS_FAILED(mStatus)) {
+      char *l_data;
+      char l_string[256];
+      int len = aSpec.Length();
+      int upper = len > 255 ? 255: len;
+      l_data =  ToNewCString(aSpec);
+      bcopy(l_data, l_string, upper);
+      l_string[upper] = '\0';
+
+      fprintf(stderr,"{debug} SetSpec %s : aSpec=%s\n", NS_FAILED(mStatus) ? "failed." : "succeeded.", l_string);
+    }
+#endif
     return *this;
   }
   NS_MutateURI& SetScheme(const nsACString& aScheme)
   {
     if (NS_FAILED(mStatus)) {
       return *this;
     }
     mStatus = mMutator->SetScheme(aScheme, nullptr);

```
The following is the list of failures logged on June 6 2020. Obviously there are bad callers who forget to attach appropriate "protocol". But the error is basically ignored by TB and SOMETHING is done subsequently.
I have a serious doubt if I should trust a mailer whose code does this underneath. But since I have been using TB for a long time, I wish I get rid of such errors and dubious behavior once for all. (Unfortuantely, since TB SEEMS TO WORK FOR ME (famous last words), I have not gotten around to fixing these.
The list also shows the protocols used for successful call. I wish they are all documented clearly.
```
========================================
SetSpec Failures/Successes
========================================

Failure first.

    504 SetSpec failed. : aSpec=branding/brand.ftl
    440 SetSpec failed. : aSpec=messenger/preferences/preferences.ftl
    234 SetSpec failed. : aSpec=calendar/preferences.ftl
    220 SetSpec failed. : aSpec=messenger/preferences/languages.ftl
    220 SetSpec failed. : aSpec=messenger/preferences/fonts.ftl
    140 SetSpec failed. : aSpec=messenger/otr/chat.ftl
     33 SetSpec failed. : aSpec=toolkit/global/resetProfile.ftl
     33 SetSpec failed. : aSpec=toolkit/global/processTypes.ftl
     33 SetSpec failed. : aSpec=toolkit/about/aboutSupport.ftl
     33 SetSpec failed. : aSpec=messenger/aboutSupportMail.ftl
     28 SetSpec failed. : aSpec=toolkit/about/aboutAddons.ftl
     11 SetSpec failed. : aSpec=DTD/xhtml1-strict.dtd
      6 SetSpec failed. : aSpec=null
      5 SetSpec failed. : aSpec=undefined
      5 SetSpec failed. : aSpec=moz-icon://
      3 SetSpec failed. : aSpec=toolkit/about/aboutProfiles.ftl
      3 SetSpec failed. : aSpec=toolkit/about/aboutConfig.ftl
      2 SetSpec failed. : aSpec=moz-icon://chrome://calendar/content/sound.wav?size=16

successful protocol list

  15817  aSpec=about
   7422  aSpec=moz-safe-about
   5055  aSpec=mailbox
   4382  aSpec=moz-nullprincipal
    758  aSpec=data
    586  aSpec=moz-icon
    108  aSpec=mailto
     77  aSpec=moz-storage-calendar
     60  aSpec=view-source
     57  aSpec=mailbox-message
     30  aSpec=smtp
     29  aSpec=page-icon
     24  aSpec=imap
     22  aSpec=addons
     22  aSpec=addbook
     18  aSpec=pop3
     17  aSpec=news
     15  aSpec=moz-memory-calendar
      6  aSpec=predictor
      3  aSpec=javascript
      3  aSpec=http
      1  aSpec=ldap
```

Maybe the person who can answer the question comment 3 can comment on the general framework of URI handling.
(I notice that file: protocol is not in the list above. Maybe I should throw a wider net and check the return code for every call site of |SetSpec| call
during a local mochitest.)

The full list of successful URI during a local mochitest run is uploaded next.

There are many instances of "type=application/x-message-display&number=0" in the list such as
```
    2 SetSpec succeeded. : aSpec=mailbox:///NEW-SSD/moz-obj-dir/objdir-tb3/_tests/testing/mochitest/browser/comm/mail/test/browser/attachment/data/bug1358565.eml?type=application/x-message-display&number=0
```
(In reply to Kai Engert (:KaiE:) from comment #4)
> Who might know the answer to comment 3 ?

I throw in another question/comment here.

Many months (a few years) ago, I noticed strange errors dumped from SetSpec() call.
Some URIs that were passed to it were not acceptable, it complained.
But TB chugged along.
I believe I entered a bugzilla about it, but unfortunately,
somehow the error message dump code was removed. Basically it was hidden under the rug.
And I cannot find the bugzilla easily.
I doubt the wisdom of removing it.

Anyway, I have maintained the following local patch to dump the error of SetSpec() in one place for my local testing.
That logging is done very aptly in a file called nsIURIMutator.idl.
This URI mutation and what protocols are valid and what are not should be documented clearly for TB maintenance.

I used to log the SUCCESS case as well in my local dump because I could not figure out what "protocol" are accepted by TB (and presumably FF, but FF does not need to grok pop3, imap, mailbox, etc.).
Here is the patch to log only the failed case. There is a protection that protects the code from an arbitrarily long data. (That is why we see a large data or URI cut short in a log I am about to upload.) 
```
# HG changeset patch
# User ISHIKAWA, Chiaki <ishikawa@yk.rim.or.jp>
# Parent  f8e3af9ab87255463730e08a419b56b1fa1c1135
dump aSpec(i.e., path) for SetSpec failure

diff --git a/netwerk/base/nsIURIMutator.idl b/netwerk/base/nsIURIMutator.idl
--- a/netwerk/base/nsIURIMutator.idl
+++ b/netwerk/base/nsIURIMutator.idl
@@ -354,16 +354,29 @@ public:
   }
 
   NS_MutateURI& SetSpec(const nsACString& aSpec)
   {
     if (NS_FAILED(mStatus)) {
       return *this;
     }
     mStatus = mMutator->SetSpec(aSpec, nullptr);
+#if DEBUG
+    if (NS_FAILED(mStatus)) {
+      char *l_data;
+      char l_string[256];
+      int len = aSpec.Length();
+      int upper = len > 255 ? 255: len;
+      l_data =  ToNewCString(aSpec);
+      bcopy(l_data, l_string, upper);
+      l_string[upper] = '\0';
+
+      fprintf(stderr,"{debug} SetSpec %s : aSpec=%s\n", NS_FAILED(mStatus) ? "failed." : "succeeded.", l_string);
+    }
+#endif
     return *this;
   }
   NS_MutateURI& SetScheme(const nsACString& aScheme)
   {
     if (NS_FAILED(mStatus)) {
       return *this;
     }
     mStatus = mMutator->SetScheme(aScheme, nullptr);

```
The following is the list of failures logged on June 6 2020. Obviously there are bad callers who forget to attach appropriate "protocol". But the error is basically ignored by TB and SOMETHING is done subsequently.
I have a serious doubt if I should trust a mailer whose code does this underneath. But since I have been using TB for a long time, I wish I get rid of such errors and dubious behavior once for all. (Unfortuantely, since TB SEEMS TO WORK FOR ME (famous last words), I have not gotten around to fixing these.
The list also shows the protocols used for successful call. I wish they are all documented clearly.
```
========================================
SetSpec Failures/Successes
========================================

Failure first.

    504 SetSpec failed. : aSpec=branding/brand.ftl
    440 SetSpec failed. : aSpec=messenger/preferences/preferences.ftl
    234 SetSpec failed. : aSpec=calendar/preferences.ftl
    220 SetSpec failed. : aSpec=messenger/preferences/languages.ftl
    220 SetSpec failed. : aSpec=messenger/preferences/fonts.ftl
    140 SetSpec failed. : aSpec=messenger/otr/chat.ftl
     33 SetSpec failed. : aSpec=toolkit/global/resetProfile.ftl
     33 SetSpec failed. : aSpec=toolkit/global/processTypes.ftl
     33 SetSpec failed. : aSpec=toolkit/about/aboutSupport.ftl
     33 SetSpec failed. : aSpec=messenger/aboutSupportMail.ftl
     28 SetSpec failed. : aSpec=toolkit/about/aboutAddons.ftl
     11 SetSpec failed. : aSpec=DTD/xhtml1-strict.dtd
      6 SetSpec failed. : aSpec=null
      5 SetSpec failed. : aSpec=undefined
      5 SetSpec failed. : aSpec=moz-icon://
      3 SetSpec failed. : aSpec=toolkit/about/aboutProfiles.ftl
      3 SetSpec failed. : aSpec=toolkit/about/aboutConfig.ftl
      2 SetSpec failed. : aSpec=moz-icon://chrome://calendar/content/sound.wav?size=16
```

The following is the successful protocol list:
```
  15817  aSpec=about
   7422  aSpec=moz-safe-about
   5055  aSpec=mailbox
   4382  aSpec=moz-nullprincipal
    758  aSpec=data
    586  aSpec=moz-icon
    108  aSpec=mailto
     77  aSpec=moz-storage-calendar
     60  aSpec=view-source
     57  aSpec=mailbox-message
     30  aSpec=smtp
     29  aSpec=page-icon
     24  aSpec=imap
     22  aSpec=addons
     22  aSpec=addbook
     18  aSpec=pop3
     17  aSpec=news
     15  aSpec=moz-memory-calendar
      6  aSpec=predictor
      3  aSpec=javascript
      3  aSpec=http
      1  aSpec=ldap
```

Maybe the person who can answer the question comment 3 can comment on the general framework of URI handling.
(I notice that file: protocol is not in the list above. Maybe I should throw a wider net and check the return code for every call site of |SetSpec| call
during a local mochitest.)

The full list of successful URI during a local mochitest run is uploaded next.

There are many instances of "type=application/x-message-display&number=0" in the list such as
```
    2 SetSpec succeeded. : aSpec=mailbox:///NEW-SSD/moz-obj-dir/objdir-tb3/_tests/testing/mochitest/browser/comm/mail/test/browser/attachment/data/bug1358565.eml?type=application/x-message-display&number=0
```

Back to Bug 1635648 Comment 5