Closed Bug 793118 Opened 12 years ago Closed 4 years ago

FileLink creates a link and attaches a file to the message too if drag'n'drop a file from a network share

Categories

(Thunderbird :: FileLink, defect)

Product:
Thunderbird
Component:
FileLink
Version:
15 Branch
Platform:
x86
Windows 7
Type:
defect
Priority:
Not set
Severity:
major

Tracking

(thunderbird_esr68 fixed, thunderbird_esr78 fixed)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 79.0
Tracking Flags:
Tracking Status
thunderbird_esr68 --- fixed
thunderbird_esr78 --- fixed

People

(Reporter: roman.yepishev, Assigned: rnons)

References

Details

Attachments

(2 files, 3 obsolete files)

2020-06-29 11_35_24-Inbox - jorgk - Mozilla Thunderbird.png
6.78 KB, image/png
Details
793118.patch
4.67 KB, patch
mkmelin
: review+
wsmwk
: approval-comm-esr68+
wsmwk
: approval-comm-esr78+
Details | Diff | Splinter Review 
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:15.0) Gecko/20100101 Firefox/15.0.1
Build ID: 20120907073726

Steps to reproduce:

Installed Thunderbird 15.0.1 en_US Win32 on a freshly installed Windows 7 Ultimate English version. Added Hotmail and YouSendIt account. Tried sending a file of 4Mb, was prompted to use FileLink. The file was uploaded to YouSendIt and a link was added to the message. Then I sent the message.


Actual results:

The file was both attached to the message AND a link to YouSendIt (where the file was previously uploaded) was in the message - http://www.youtube.com/watch?v=B4g3Hw3pTa0
The only error in the Error Console was this - Security Error: Content at moz-nullprincipal:{bf02df00-af4a-4431-9020-f40b79e683a3} may not load or link to about:blank.


Expected results:

The file should no longer be attached to the message.
OS: Linux → Windows 7
Hardware: x86_64 → x86
Hi,

I have Thunderbird on a vista and a 7 machine and I have the same problem.
The Thunderbird version is 15.0.1 Fr.
Thanks
Hm.

Are either of you able to reproduce this error sending those files to yourself?
I've just tryed and the problem seems to be corrected. Today it works, but wednesday my message contains the joined file and the YouSendIt link.

I'm sorry, I didn't try before posting, but I didn't see any Thunderbird upgrade this week.
Yes, I sent the file to myself and the file got attached too.
What's interesting is that I can't reproduce this neither on my Ubuntu machine nor on a different Win7 Ultimate VM created for different purposes.

The installation was performed from this URL - http://releases.mozilla.org/pub/mozilla.org/thunderbird/releases/15.0.1/win32/en-US/Thunderbird%20Setup%2015.0.1.exe and the MD5 sum matches the one distributed via CDN so I am quite puzzled about what is different about this VM and this installation.
I think I need some debugging information.

Could you please go to your Preferences dialog (Tools > Options on Windows), go to the Advanced pane, and click on "Config Editor" under the "General" tab.

Right click on the list of prefs, and choose New > String.

The (case-sensitive) pref name you want to add is: YouSendIt.logging.console

The (case-sensitive) value you want to give this pref is: All

Then restart Thunderbird. Open up the Error Console (Tools > Error Console).

There might already be stuff in the Error Console. Remove it by clicking "clear" in the Error Console window.

Then try to reproduce your error. Logging information should appear in the Error Console (make sure your Error Console is viewing "All" to see all of the messages).

When you reproduce your problem, do any errors crop up? If so, paste them in here.
Note that in at least one log message, account information might get sent to the log console. This won't be passwords or anything, but it'll be stuff like space remaining, your email address, etc. Please don't paste that in the bug, unless you're comfortable sharing it.
No, there are no errors. There are the last lines in the log:

2012-09-21 08:42:49	YouSendIt	INFO	findDownloadUrl response = {"name":"Good Song (Roman Yepishev).ogg","id":"70966631","size":"3327625","revision":"1","ownedByStorage":"11348372","createdOn":"2012-09-21T15:42:48","parentid":"21600528","downloadUrl":"/dpi/v1/folder/file/download/U...redacted... w","clickableDownloadUrl":"http://www.yousendit.com/download/T...redactedw"}
Timestamp: 9/21/2012 8:42:53 AM
Warning: Use of attributes' nodeValue attribute is deprecated. Use value instead.
Source File: chrome://messenger/content/messengercompose/MsgComposeCommands.js
Line: 3986
Security Error: Content at moz-nullprincipal:{9dc34abb-c670-4381-a7a1-4544057146eb} may not load or link to about:blank. (repeats twice in the log)

I have to note that when I try uploading the file to Ubuntu One the same issue happens so that does not look like a YouSendIt-specific issue.
Roman:

Hm. Unable to reproduce using Ubuntu, Thunderbird 15.0.1.

Does this problem persist with a particular file? Perhaps Thunderbird is choking on special characters in the filename...

-Mike
Exactly, can't reproduce this in Ubuntu or on another Windows 7 machine too.
Even sample file from windows 7, Kalimba.mp3 remains attached and it does not look like a bad filename.
Is there any way to get more debug info from the whole FileLink subsystem?
Roman:

Hm. No dice on my Windows 7 machine either.

Can you try reproducing with add-ons disabled?  Help > Restart with add-ons disabled.

-Mike
Restarted with add-ons disabled - the file linked properly.
Re-enabled the add-ons - the file linked properly too.


And now I can't reproduce the issue even after deleting the thunderbird data from %LOCALAPPDATA% and %APPDATA%. Invalid? There's definitely something wrong but now I can't make it fail.
Spooky. :/ I don't like bugs that we can't reproduce reliably... they freak me out.
The problem must come from the autosave of drafts.
When I load a file via YouSendIt and transmission time exceeds the time of autosave, an email containing the attachment is saved in drafts.
for information, a French user reports a similar problem with U1 account and TB17. See: http://forums.mozfr.org/viewtopic.php?f=4&t=109377
(In reply to Vincent (caméléon) from comment #14)
> for information, a French user reports a similar problem with U1 account and
> TB17. See: http://forums.mozfr.org/viewtopic.php?f=4&t=109377

His problem is solved after disabling draft auto-save all the x minutes. Does it helps to understand how this issue occur?
Another report of this problem with a different workaround:
"I've discovered the source of the problem. If you attach directly from a network share (\\192.168.x.x\blah\abz.jpg), Thunderbird won't remove the attachments from the email message by using Filelink. "

from https://getsatisfaction.com/mozilla_messaging/topics/filelink_bug_uploaded_attachments_arent_removed_from_email
Same problem with TB17, XP SP3.  Still happens with Add-ons disabled.  Don't know enough to understand what "attaching directly from a network share" means.
This bug is also affecting me and I can confirm that it occurs when attaching a file from a network share (a NAS drive in my case). The same file attached from the local hard disk does not cause a problem. However, for my customers this is not a practical workaround so this bug is quite significant for them.
(In reply to Andy from comment #18)
> This bug is also affecting me and I can confirm that it occurs when
> attaching a file from a network share (a NAS drive in my case). The same
> file attached from the local hard disk does not cause a problem. However,
> for my customers this is not a practical workaround so this bug is quite
> significant for them.

I can also confirm this. 
Thunderbird 17.0.6 on Windows 8 x64. 
Dropbox FileLink add-on
Attach a file from a Windows network share. 
Click "Link"
The attachment correctly uploads, the FileLink text is added to the email body, and the attachment icon changes.
But when you send the email, the full attachment is still sent with the email.
I attached by drag-and-drop from Windows Explorer. File was 7.40MB .7z extension.
Server is running Windows Server 2012 Standard.

Attach the same file form a local disk, and it all works as expected.
Same here, TB 17.0.7 on win XP SP3. Filelinking from network source same as described by steve at 13/06/18.
Same problem with TB 24.2.0 on win7 home premium SP1.  Dropbox FileLinked attachment is not removed when attached directly from a network share (NAS), but is removed if attached from a drive letter mapped to the same network share.
so autosave or manual save of a draft is not a necessary step?
Flags: needinfo?(smog1)
Flags: needinfo?(sbriels)
Flags: needinfo?(gorlen)
I had not verified if the files came from the network or hard drive, sorry.
I made new tests and yes, just network files are problematic. A linked file from the hard disk does not pose any problems. I tried the manual and automatic backup from a local file and everything goes well.
Flags: needinfo?(smog1)
sorry, don't have that setup anymore (machine was at my previous employer) so cannot check details. Problem was that attaching files as yousendit or box filelink (dont remember exactly) from a network drive would fail.
Flags: needinfo?(sbriels)
Same problem. TB 38.0.1, Win7 Pro SP1.

The problem occurs only when I drag'n'drop a file from a network share. If the share is mounted as the disc, or when I add a file from network share via menu, the problem does not occur.

So, a workaround is to mount the network share as a drive.
reproduces using nightly build. just drag and drop
Severity: normal → major
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(gorlen)
Summary: FileLink creates a link and attaches a file to the message too → FileLink creates a link and attaches a file to the message too if drag'n'drop a file from a network share
(In reply to Aleksey from comment #26)
> Same problem. TB 38.0.1, Win7 Pro SP1.
> 
> The problem occurs only when I drag'n'drop a file from a network share. If
> the share is mounted as the disc, or when I add a file from network share
> via menu, the problem does not occur.
> 
> So, a workaround is to mount the network share as a drive.

Yes I know about this workaround. But in my case TB is used by lot of differents users, using lot of differents shared folders, it's not possible to mount all of them as a netowrk share. I use it punctually.
Same problem on thunderbird 60, windows 10.
I think is a duplicate of bug 1051800

Reproduced with Thunderbird 68.9.0 on Windows 7 with mapped windows share (mapped to a driver letter). See also bug 1640642

Reproduced with Thunderbird 68.9.0 on Windows 10 with both mapped and unmapped windows share and different Filelink providers (*cloud, Filelink for Plik). The "auto-save draft" option doesn't change the behavior.

So, further tests show different behaviour in subtly different scenarios (all on an IMAP mail account, in case it makes a difference):

  1. Attach as a regular attachment from letter-mapped network drive, then convert to linked file: works correctly
  2. Directly link file from letter-mapped network drive: both linked and attached
    1. Remove attachment before trying to send email: the link is removed with the attachment
    2. First try to send email, get "message too big" warning, cancel sending, then remove attachments: the links stay; this can serve as work-around.
  3. Link in any way (directly or by conversion) from UNC network path (\server\path\to\file): both linked and attached. Subcases 3.i and 3.ii as in 2.i and 2.ii
  4. Link in any way (directly or by conversion) from local filesystem: works correctly

I'd like to set a bug bounty for this bug. Is that generally accepted among Thunderbird developers? What is the preferred platform?
TIA

Hallo Walts48,
why is this bug not in status confirmed... I think it is deplorable to see quite a few people reproducing this bug and still pretending it is NEW (for 8 Years now).

Flags: needinfo?(wls220spring)

I can confirm that an attachment from a windows share over UNC paths does also attach the file, which makes EVERY FileLinkProvider useless.

I can also confirm, that if the file is bigger than the max-mailsize-limit of the mail server the file-link "stays" in the mail body when the attachment is manually removed after getting a pop-up that the server might not send the mail because of size limit.

Please fix this nasty bug.

(In reply to Heinrich Hartl from comment #38)

Hallo Walts48,
why is this bug not in status confirmed... I think it is deplorable to see quite a few people reproducing this bug and still pretending it is NEW (for 8 Years now).

Confirmed and set to NEW in Comment 27.

Flags: needinfo?(wls220spring)

(In reply to Johannes Endres from comment #36)

I'd like to set a bug bounty for this bug. Is that generally accepted among Thunderbird developers? What is the preferred platform?

I know one case where this has been done: bug 394216 comment #142 (I collected that and donated it to a good cause).

Do we have Windows developers these days? Ping?

Flags: needinfo?(remotenonsense)

I do have a Windows machine, I will see if I can reproduce next week.

Flags: needinfo?(remotenonsense)
Attached patch 793118.patch (obsolete) — — Splinter Review

If a network shared file is sent via cloud, handle it the same as a comp field local attachment so it won't be sent as a normal attachment. I didn't dig deep, but I tested it works on Windows.

Attachment #9159971 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9159971 [details] [diff] [review]
793118.patch

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

::: mailnews/compose/src/nsMsgCompUtils.cpp
@@ +1306,5 @@
>      rv = pNetService->NewURI(aSpec, nullptr, nullptr, aInstancePtrResult);
>    return rv;
>  }
>  
> +bool nsMsgIsLocalFile(const char* url, bool sendViaCloud) {

I don't thin you should mix in sendViaCloud in this function. Looks like you can just use the variable everywhere locally instead of passing it in, no?
Attachment #9159971 - Flags: review?(mkmelin+mozilla)
Assignee: nobody → remotenonsense
Status: NEW → ASSIGNED
Attached patch 793118.patch (obsolete) — — Splinter Review

Updated. Seems a bit different, I'm not sure if it's possible that on non-Windows platform, nsMsgIsLocalFile returns false, but sendViaCloud is true. But even if that's the case, perhaps it's still good to handle it as local.

Attachment #9159971 - Attachment is obsolete: true
Attachment #9159991 - Flags: review?(mkmelin+mozilla)

Why don't you just fix nsMsgIsLocalFile(), it's only used in nsMsgSend.cpp:
https://searchfox.org/comm-central/search?q=nsMsgIsLocalFile&path=&case=false&regexp=false
And while your there, move the function to nsMsgSend.cpp or even inline it.

I have no idea what "not really local" is meant to mean. On all other platforms stuff that lives on some server miles away is considered local, but UNC path on Windows are treated differently. IMHO, that makes no sense.

Why don't you just fix nsMsgIsLocalFile(), it's only used in nsMsgSend.cpp:

Yes, I considered removing these 3 lines at first https://searchfox.org/comm-central/source/mailnews/compose/src/nsMsgCompUtils.cpp#1315-1317. That is enough to fix the current bug. But I was not sure if it will break something else.

I'd be inclined to do that since IMHO the distinction makes no sense. Of course you change the behaviour, when not using CloudFile, on Windows you will attach the file instead of sending a link to the server. Is that correct? I thought the complaint is that files via UNC path don't get uploaded and are included instead. But what happens with small files via UNC now? Do they get attached or does a link to the server get sent?

So can you state the exact behaviour for small and big files via UNC path and then let us how that changes if the three lines were removed.

If you change behaviour, maybe people will get upset. In which case there could be an option: "treat UNC path as local".

I suppose the intention here may have been that small companies could send "attachments" with UNC links instead of actual attachments, for internal usage? Not sure if it worked as intended, but doesn't seem all that useful mixing up what is an attachment or not...

(In reply to Magnus Melin [:mkmelin] from comment #49)

I suppose the intention here may have been that small companies could send "attachments" with UNC links instead of actual attachments, for internal usage?

Right, but does this work? If it works, maybe we shouldn't change the behaviour.

OK, I tried it, small file via UNC path. That gets attached. So as far as I can tell, removing the three lines will fix the issue without negative side effects.

(In reply to Jorg K (CEST = GMT+2) from comment #50)

Right, but does this work? If it works, maybe we shouldn't change the behaviour.

Without checks that it's sending so someone internal it seems pretty bad. Say you have a company shared UNC disk where you keep up to date offers or something like this, then try to send it to the external customer...

Well, doesn't seem to work, bad idea, so remove it, right?

(In reply to Jorg K (CEST = GMT+2) from comment #48)

I thought the complaint is that files via UNC path don't get uploaded and are included instead.

No, that's not the problem. Actually the file is uploaded (and a link put into the message) and the file is additionally included in the mail.

Thanks for the clarification, I got a bit lost here. So it seems that trying to treat the UNC path especially only causes issues and might was well be removed.

(In reply to Jorg K (CEST = GMT+2) from comment #55)

Thanks for the clarification, I got a bit lost here. So it seems that trying to treat the UNC path especially only causes issues and might was well be removed.

I very strongly second this suggestion ;-)

Attached patch 793118.patch (obsolete) — — Splinter Review

Removed special handling of UNC file url on Windows.

Attachment #9159991 - Attachment is obsolete: true
Attachment #9159991 - Flags: review?(mkmelin+mozilla)
Attachment #9160026 - Flags: review?(mkmelin+mozilla)

Well, the entire function then becomes: return PL_strncasecmp(url, "file://", 7) == 0. As I said before, you should inline that.

Attached patch 793118.patch — — Splinter Review

You're right, changed to use PL_strncasecmp directly in nsMsgSend. Or do you mean turn it into an inline function? nsMsgSend.cpp will be rewrote in bug 1211292, so I think call PL_strncasecmp is good for now.

Attachment #9160026 - Attachment is obsolete: true
Attachment #9160026 - Flags: review?(mkmelin+mozilla)
Attachment #9160047 - Flags: review?(mkmelin+mozilla)

Yes, this is what I meant.

Comment on attachment 9160047 [details] [diff] [review]
793118.patch

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

LGTM, r=mkmelin
Attachment #9160047 - Flags: review?(mkmelin+mozilla) → review+
status-thunderbird78: --- → affected
status-thunderbird_esr78: --- → affected
Keywords: checkin-needed-tb
Target Milestone: --- → Thunderbird 79.0

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/1c14caeabdd5
Do not send network shared file as normal attachment when already uploded to filelink. r=mkmelin,jorgk

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Keywords: checkin-needed-tb
Resolution: --- → FIXED

Is it possible to download a testbuild for Windows x86+x64?
Will this fix land in TB 68.9.X?

Is it possible to download a testbuild for Windows x86+x64?

You can download a nightly build later today from http://ftp.mozilla.org/pub/thunderbird/nightly/2020/06/. Look for 2020-06-30 at the bottom, 06-29 doesn't contain this patch.

Will this fix land in TB 68.9.X?

I guess unlikely.

Comment on attachment 9160047 [details] [diff] [review]
793118.patch

Simple patch, low risk, aligns behaviour on all platforms. Long-standing annoying bug, should fix eventually.
Attachment #9160047 - Flags: approval-comm-esr78?
Attachment #9160047 - Flags: approval-comm-esr68?

(In reply to Ping Chen (:rnons) from comment #64)

Is it possible to download a testbuild for Windows x86+x64?

You can download a nightly build later today from http://ftp.mozilla.org/pub/thunderbird/nightly/2020/06/. Look for 2020-06-30 at the bottom, 06-29 doesn't contain this patch.

Will this fix land in TB 68.9.X?

I guess unlikely.

Thank you.

(In reply to Jorg K (CEST = GMT+2) from comment #65)

Comment on attachment 9160047 [details] [diff] [review]
793118.patch

Simple patch, low risk, aligns behaviour on all platforms. Long-standing
annoying bug, should fix eventually.

Hello Jorg K,
I don't have the time and resources to compile TB for Windows (multiplatform) for every new security update coming up until ESR78 ist out.
The point is why to wait for ESR78? Some people are already waiting more than 8 years. This Bug has 3 Duplicates! Why?

And you already mentioned... "low risk".
My employer is using TB for over 300 users. This patch ist "critical" for us.

I don't understand the comment. I requested inclusion in TB 68.x, it's too late for TB 68.10 which is being built at time of writing.

(In reply to Jorg K (CEST = GMT+2) from comment #68)

I don't understand the comment. I requested inclusion in TB 68.x, it's too late for TB 68.10 which is being built at time of writing.

Sorry, I was confused by reading "Tracking/Tracking Flags > Milestone: Thunderbird 79.0"
Thank you... waiting excitedly for 68.10 then.

Uuupps, I meant 68.11.

Comment on attachment 9160047 [details] [diff] [review]
793118.patch

Approved for esr78
Attachment #9160047 - Flags: approval-comm-esr78? → approval-comm-esr78+
Comment on attachment 9160047 [details] [diff] [review]
793118.patch

Approved for esr68

[Triage Comment]
Attachment #9160047 - Flags: approval-comm-esr68? → approval-comm-esr68+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: