Thunderbird broken by mozilla-central Bug 1143922 nsPop3Protocol.obj : error LNK2001: unresolved external symbol "public: virtual enum nsresult __stdcall nsMsgProtocol::Open2(class nsIInputStream * *)" (?Open2@nsMsgProtocol@@UAG?AW4nsresult@@PAPAVnsIInput

RESOLVED FIXED in Thunderbird 42.0

Status

defect
--
blocker
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: philip.chee, Assigned: ewong)

Tracking

({dogfood})

Trunk
Thunderbird 42.0

Firefox Tracking Flags

(firefox42 affected, thunderbird42 fixed)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

4 years ago
nsPop3Protocol.obj : error LNK2001: unresolved external symbol "public: virtual enum nsresult __stdcall nsMsgProtocol::Open2(class nsIInputStream * *)" (?Open2@nsMsgProtocol@@UAG?AW4nsresult@@PAPAVnsIInputStream@@@Z)
nsNNTPProtocol.obj : error LNK2001: unresolved external symbol "public: virtual enum nsresult __stdcall nsMsgProtocol::Open2(class nsIInputStream * *)" (?Open2@nsMsgProtocol@@UAG?AW4nsresult@@PAPAVnsIInputStream@@@Z)
nsMsgProtocol.obj : error LNK2001: unresolved external symbol "public: virtual enum nsresult __stdcall nsMsgProtocol::Open2(class nsIInputStream * *)" (?Open2@nsMsgProtocol@@UAG?AW4nsresult@@PAPAVnsIInputStream@@@Z)
nsSmtpProtocol.obj : error LNK2001: unresolved external symbol "public: virtual enum nsresult __stdcall nsMsgProtocol::Open2(class nsIInputStream * *)" (?Open2@nsMsgProtocol@@UAG?AW4nsresult@@PAPAVnsIInputStream@@@Z)
nsImapProtocol.obj : error LNK2001: unresolved external symbol "public: virtual enum nsresult __stdcall nsMsgProtocol::Open2(class nsIInputStream * *)" (?Open2@nsMsgProtocol@@UAG?AW4nsresult@@PAPAVnsIInputStream@@@Z)
nsMailboxProtocol.obj : error LNK2001: unresolved external symbol "public: virtual enum nsresult __stdcall nsMsgProtocol::Open2(class nsIInputStream * *)" (?Open2@nsMsgProtocol@@UAG?AW4nsresult@@PAPAVnsIInputStream@@@Z)
nsPop3Protocol.obj : error LNK2001: unresolved external symbol "public: virtual enum nsresult __stdcall nsMsgProtocol::AsyncOpen2(class nsIStreamListener *)" (?AsyncOpen2@nsMsgProtocol@@UAG?AW4nsresult@@PAVnsIStreamListener@@@Z)
nsNNTPProtocol.obj : error LNK2001: unresolved external symbol "public: virtual enum nsresult __stdcall nsMsgProtocol::AsyncOpen2(class nsIStreamListener *)" (?AsyncOpen2@nsMsgProtocol@@UAG?AW4nsresult@@PAVnsIStreamListener@@@Z)
nsMsgProtocol.obj : error LNK2001: unresolved external symbol "public: virtual enum nsresult __stdcall nsMsgProtocol::AsyncOpen2(class nsIStreamListener *)" (?AsyncOpen2@nsMsgProtocol@@UAG?AW4nsresult@@PAVnsIStreamListener@@@Z)
nsSmtpProtocol.obj : error LNK2001: unresolved external symbol "public: virtual enum nsresult __stdcall nsMsgProtocol::AsyncOpen2(class nsIStreamListener *)" (?AsyncOpen2@nsMsgProtocol@@UAG?AW4nsresult@@PAVnsIStreamListener@@@Z)
nsImapProtocol.obj : error LNK2001: unresolved external symbol "public: virtual enum nsresult __stdcall nsMsgProtocol::AsyncOpen2(class nsIStreamListener *)" (?AsyncOpen2@nsMsgProtocol@@UAG?AW4nsresult@@PAVnsIStreamListener@@@Z)
nsMailboxProtocol.obj : error LNK2001: unresolved external symbol "public: virtual enum nsresult __stdcall nsMsgProtocol::AsyncOpen2(class nsIStreamListener *)" (?AsyncOpen2@nsMsgProtocol@@UAG?AW4nsresult@@PAVnsIStreamListener@@@Z)
nsImapProtocol.obj : error LNK2001: unresolved external symbol "public: virtual enum nsresult __stdcall nsImapMockChannel::Open2(class nsIInputStream * *)" (?Open2@nsImapMockChannel@@UAG?AW4nsresult@@PAPAVnsIInputStream@@@Z)
nsImapProtocol.obj : error LNK2001: unresolved external symbol "public: virtual enum nsresult __stdcall nsImapMockChannel::AsyncOpen2(class nsIStreamListener *)" (?AsyncOpen2@nsImapMockChannel@@UAG?AW4nsresult@@PAVnsIStreamListener@@@Z)
nsNntpMockChannel.obj : error LNK2001: unresolved external symbol "public: virtual enum nsresult __stdcall nsNntpMockChannel::Open2(class nsIInputStream * *)" (?Open2@nsNntpMockChannel@@UAG?AW4nsresult@@PAPAVnsIInputStream@@@Z)
nsNntpMockChannel.obj : error LNK2001: unresolved external symbol "public: virtual enum nsresult __stdcall nsNntpMockChannel::AsyncOpen2(class nsIStreamListener *)" (?AsyncOpen2@nsNntpMockChannel@@UAG?AW4nsresult@@PAVnsIStreamListener@@@Z)
xul.dll : fatal error LNK1120: 6 unresolved externals
c:/t1/hg/comm-central/mozilla/config/rules.mk:826: recipe for target 'xul.dll' failed
Severity: normal → blocker
Keywords: dogfood
(Reporter)

Comment 1

4 years ago
Posted patch Patch v1.0 Proposed Fix. (obsolete) — Splinter Review
I'm just guessing that we don't really need those security checks so I've added Open2() and AsyncOpen2() that just forwards to Open() and AsyncOpen()
Assignee: nobody → philip.chee
Status: NEW → ASSIGNED
Attachment #8636260 - Flags: review?(Pidgeot18)
(Reporter)

Updated

4 years ago
Attachment #8636260 - Flags: review?(Pidgeot18)
(Reporter)

Comment 2

4 years ago
Anyone wants to take over this bug please feel free. I've run out of time.
Assignee: philip.chee → nobody
Status: ASSIGNED → NEW
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
(Assignee)

Comment 19

4 years ago
Posted patch proposed patch (v1) (wip) (obsolete) — Splinter Review
Assignee: nobody → ewong
Status: NEW → ASSIGNED
(Assignee)

Comment 20

4 years ago
Comment on attachment 8636352 [details] [diff] [review]
proposed patch (v1) (wip)

Review of attachment 8636352 [details] [diff] [review]:
-----------------------------------------------------------------

::: mailnews/base/util/nsMsgProtocol.cpp
@@ +514,5 @@
>  }
>  
> +NS_IMETHODIMP nsMsgProtocol::Open2(nsIInputStream **_retval)
> +{
> +  nsCOMPtr<nsIStreamListner> listener;

nsIStreamListner  will be changed to 
nsIStreamListener.
(Assignee)

Comment 21

4 years ago
Posted patch proposed patch(v2) (obsolete) — Splinter Review
Attachment #8636260 - Attachment is obsolete: true
Attachment #8636352 - Attachment is obsolete: true
Attachment #8636367 - Flags: review?(rkent)
(Assignee)

Comment 22

4 years ago
Comment on attachment 8636367 [details] [diff] [review]
proposed patch(v2)

No.. wrong..  sorry for the bugspam.
Attachment #8636367 - Flags: review?(rkent)

Comment 23

4 years ago
And now my build succeeds. SM-Trunk Linux x86_64
(Assignee)

Comment 24

4 years ago
I'm not exactly sure how to approach this as I'm not familiar with this code.
While the 
From: http://mxr.mozilla.org/comm-central/source/mailnews/imap/src/nsImapService.cpp#453

440    if (NS_SUCCEEDED(rv) && aStreamListener)
441     {
442       nsCOMPtr<nsIChannel> aChannel;
443       nsCOMPtr<nsILoadGroup> aLoadGroup;
444       nsCOMPtr<nsIMsgMailNewsUrl> mailnewsUrl = do_QueryInterface(uri, &rv);
445       if (NS_SUCCEEDED(rv) && mailnewsUrl)
446         mailnewsUrl->GetLoadGroup(getter_AddRefs(aLoadGroup));
447 
448       rv = NewChannel(uri, getter_AddRefs(aChannel));
449       NS_ENSURE_SUCCESS(rv, rv);
450 
451       nsCOMPtr<nsISupports> aCtxt = do_QueryInterface(uri);
452       //  now try to open the channel passing in our display consumer as the listener
453       return aChannel->AsyncOpen(aStreamListener, aCtxt);
454     }
455   }

So as I understand it from the Firefox code, there's a security check
within the AsyncOpen2() code and then it calls AsyncOpen with the ctx as
nullptr. 

In the above case, what is the right approach for this? If I just blindly
replace the above with AsyncOpen2(), we'd lose the point of setting up the
aCtxt. 

There are a couple of instances of the above in nsImapService.cpp and
nsMsgQuote.cpp.
Flags: needinfo?(rkent)
(In reply to Edmund Wong (:ewong) from comment #24)
> In the above case, what is the right approach for this? If I just blindly
> replace the above with AsyncOpen2(), we'd lose the point of setting up the
> aCtxt. 

For the time being, I would advocate just adding the AsyncOpen2/Open2 methods and not trying to switch to them.
Flags: needinfo?(rkent)
(Assignee)

Comment 26

4 years ago
(In reply to Joshua Cranmer [:jcranmer] from comment #25)
> (In reply to Edmund Wong (:ewong) from comment #24)
> > In the above case, what is the right approach for this? If I just blindly
> > replace the above with AsyncOpen2(), we'd lose the point of setting up the
> > aCtxt. 
> 
> For the time being, I would advocate just adding the AsyncOpen2/Open2
> methods and not trying to switch to them.

Thanks.
(Assignee)

Comment 27

4 years ago
Posted patch proposed patch(v3) (obsolete) — Splinter Review
Attachment #8636367 - Attachment is obsolete: true
Attachment #8636398 - Flags: review?(rkent)

Comment 28

4 years ago
Comment on attachment 8636398 [details] [diff] [review]
proposed patch(v3)

>+++ b/mailnews/base/util/nsMsgProtocol.cpp
> }
Nit: missing empty line
>+NS_IMETHODIMP nsMsgProtocol::AsyncOpen2(nsIStreamListener *aListener)
>+{
>+    nsCOMPtr<nsIStreamListener> listener = aListener;
>+    nsresult rv = nsContentSecurityManager::doContentSecurityCheck(this, listener);
>+    NS_ENSURE_SUCCESS(rv, rv);
>+    return AsyncOpen(listener, nullptr);
>+}

>+++ b/mailnews/imap/src/nsImapProtocol.cpp
>@@ -8960,22 +8961,29 @@ NS_IMETHODIMP nsImapMockChannel::SetURI(
>           if (NS_SUCCEEDED(msgHdr->GetMessageSize(&messageSize)))
>             SetContentLength(messageSize);
>         }
>       }
>     }
>   }
>   return NS_OK;
> }
>-
Why remove this line?
> NS_IMETHODIMP nsImapMockChannel::Open(nsIInputStream **_retval)
> {
>   return NS_ImplementChannelOpen(this, _retval);
> }

rs=me
Attachment #8636398 - Flags: review?(rkent) → review+

Comment 29

4 years ago
a=me for CLOSED TREE too
(Assignee)

Comment 30

4 years ago
(In reply to Ian Neal from comment #28)
> Comment on attachment 8636398 [details] [diff] [review]
> proposed patch(v3)
> 
> >+++ b/mailnews/base/util/nsMsgProtocol.cpp
> > }
> Nit: missing empty line

Fixed.

> >+NS_IMETHODIMP nsMsgProtocol::AsyncOpen2(nsIStreamListener *aListener)
> >+{
> >+    nsCOMPtr<nsIStreamListener> listener = aListener;
> >+    nsresult rv = nsContentSecurityManager::doContentSecurityCheck(this, listener);
> >+    NS_ENSURE_SUCCESS(rv, rv);
> >+    return AsyncOpen(listener, nullptr);
> >+}
> 
> >+++ b/mailnews/imap/src/nsImapProtocol.cpp
> >@@ -8960,22 +8961,29 @@ NS_IMETHODIMP nsImapMockChannel::SetURI(
> >           if (NS_SUCCEEDED(msgHdr->GetMessageSize(&messageSize)))
> >             SetContentLength(messageSize);
> >         }
> >       }
> >     }
> >   }
> >   return NS_OK;
> > }
> >-
> Why remove this line?

Sorry. Fixed.

Will update the patch and push it.
(Assignee)

Comment 31

4 years ago
Attachment #8636398 - Attachment is obsolete: true
Attachment #8636888 - Flags: review+
(Assignee)

Comment 32

4 years ago
Comment on attachment 8636888 [details] [diff] [review]
patch for checkin

Pushed to comm-central:
https://hg.mozilla.org/comm-central/rev/3903bf440cfd
(Assignee)

Updated

4 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 42.0
(Assignee)

Updated

4 years ago
You need to log in before you can comment on or make changes to this bug.