Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Download filename will be cut up to the first comma #29346

Closed
3 tasks done
sherlock1982 opened this issue May 26, 2021 · 7 comments
Closed
3 tasks done

[Bug]: Download filename will be cut up to the first comma #29346

sherlock1982 opened this issue May 26, 2021 · 7 comments
Labels

Comments

@sherlock1982
Copy link

Preflight Checklist

Electron Version

12.0.8

What operating system are you using?

Windows

Operating System Version

Windows 10

What arch are you using?

x64

Last Known Working Electron version

No response

Expected Behavior

Downloading a file with a comma in name should return full filename.

Actual Behavior

Filename is cut up to the first comma.

Testcase Gist URL

No response

Additional Information

I'm aware that filename in content-disposition should be quoted.
And I specifically checked that ASP.NET Core does it correctly.

What's interesting is that if i add additional double quotes it works (but adds additional underscores in normal browsers).

@Prinzhorn
Copy link
Contributor

Does this behave differently than Chromium? There are a lot of search results regarding that topic https://foocompelsyou.wordpress.com/2012/10/01/the-curious-case-of-chrome-content-disposition-and-the-comma/

@sherlock1982
Copy link
Author

I haven't checked Chromium to be honest. But all modern browsers are ok including Chrome.

So in case it's an error in Chromium it should be reported there?

@Prinzhorn
Copy link
Contributor

I'm not sure if Chrome and Chromium would behave differently, I don't think so. Can you provide steps to reproduce the issue in Electron? Can you host a URL that serves the problematic headers or provide steps to do that locally (e.g. this could be as simple as using netcat). And then a very basic Fiddle that triggers the problem? Otherwise the Electron team will have a hard time helping you.

@Prinzhorn
Copy link
Contributor

Prinzhorn commented May 27, 2021

I cannot reproduce this on Ubuntu.

http.txt

HTTP/1.1 200 OK
Content-Type: text/plain
Content-Disposition: attachment; filename="foo,bar.txt"
Content-Length: 3

foo

Serve via ncat on port 1337

ncat -l 1337 < http.txt

Minimal Electron code:

const {app, BrowserWindow} = require('electron')

function createWindow () {
  const mainWindow = new BrowserWindow({
    width: 800,
    height: 600
  })

  mainWindow.loadURL('http://localhost:1337/')
}

app.whenReady().then(createWindow);

Selection_901

@alefoll
Copy link

alefoll commented Sep 21, 2021

Hey, I get the same bug, all versions > 12 are impacted.
@Prinzhorn, to reproduce it you need to filter with onHeadersReceived.

http.txt

HTTP/1.1 200 OK
Content-Type: text/plain
Content-Disposition: attachment; filename="foo,bar.txt"
Content-Length: 3

foo

Serve via ncat on port 1337

ncat -l 1337 < http.txt

Minimal Electron code:

const {app, BrowserWindow} = require('electron')

function createWindow () {
  app.on('session-created', session => {
    session.webRequest.onHeadersReceived((details, callback) => {
      callback({ responseHeaders: details.responseHeaders });
    });
  });

  const mainWindow = new BrowserWindow({
    width: 800,
    height: 600
  })

  mainWindow.loadURL('http://localhost:1337/')
}

app.whenReady().then(createWindow);

@Prinzhorn
Copy link
Contributor

I can reproduce this on Ubuntu now (@nornagon) thanks @alefoll

Screenshot is 15-beta7 via Fiddle

Selection_978

The parsing seems to incorrectly strip the double quotes, which breaks serializing it back. This is logged in onHeadersReceived:

{
  'Content-Disposition': [ ' attachment; filename=foo,bar.txt' ], <========= No more double quotes
  'Content-Length': [ '3' ],
  'Content-Type': [ 'text/plain' ]
}

@alefoll
Copy link

alefoll commented Oct 1, 2021

Hey @nornagon, do you have some time to look into it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants