electron / electron

:electron: Build cross-platform desktop apps with JavaScript, HTML, and CSS
https://electronjs.org
MIT License
113.81k stars 15.32k forks source link

file scheme should send implementation defined origin header #7054

Closed 1999 closed 8 years ago

1999 commented 8 years ago

I am using Fetch API to obtain data from our internal API service. But instead of the fact that all response headers are present in devtools network panel, Fetch API Response object has only content-language and content-type headers.

Here's a screencast of that: http://imgur.com/I9jciR5

The code is pretty simple.

// background.js
win = new BrowserWindow({
  width: 800,
  height: 600
});

// ui.js
const fetchResource = async (url, token) => {
    const headers = new Headers;
    headers.set('Authorization', `OAuth ${token}`);

    try {
        return await fetch(url, {headers, mode: 'cors'});
    } catch (err) {
        pushToDebugChannel(`Fetch op failed: ${err.message}`);
        throw err;
    }
}

const res = await fetchResource(url, token);
console.log([...res.headers]);

Seems like this issue is somewhat close to #4326

deepak1556 commented 8 years ago

Do you get the expected output in chrome ? Have you verified it has Access-Control-Expose-Headers incase its a cors request ?

1999 commented 8 years ago

This is the code:

const headers = new Headers;
headers.set('Authorization', 'OAuth ...')

fetch('https://api/v2/issue/changelog', {mode: 'cors', headers}).then(res => {
  console.log(new Map([...res.headers]))
})

This code produces different headers in browser and electon-based app. The problem seems to be somewhere inside the app because {mode: 'cors'} does nothing: Origin header is not set and this is why server doesn't send Access-Control-Expose-Headers header. Am I missing smth and there is an opportunity to set Origin header by hands in electron?

deepak1556 commented 8 years ago

Can't tell whats wrong without a reproducible example or a log-net-log trace, would it be possible to provide either one ? As far as I tested origin headers are sent.

an opportunity to set Origin header by hands in electron

You can use ses.webRequest.onBeforeSendHeaders to manipulate request headers.

1999 commented 8 years ago

Sorry for waiting so long. I made a little investigation and it seems like the problem appears when the server's Access-Control-Allow-Origin header is set not just to asterisk, but when the server also checks the Origin of request and sets response header value to request origin if it's valid.

I wrote a simple express code for that and use it in my gist:

app.get('/_cors', function (req, res, next) {
    if (req.headers.origin) {
        res.set({
            'Access-Control-Allow-Credentials': true,
            'Access-Control-Allow-Headers': 'Authorization, If-None-Match, If-Match, Accept, If-Modified-Since, Accept-Encoding, Accept-Language, Content-Type, If-Unmodified-Since',
            'Access-Control-Allow-Origin': req.headers.origin,
            'Access-Control-Expose-Headers': 'ETag,Content-Encoding,Vary,Last-Modified,Content-Language,Link,Content-Type'
        });
    }

    res.json({
        request_headers: req.headers
    });
});

https://gist.github.com/1999/83a4a55b417cfb5ca046a62a36d2ae70

1) I expect 2 headers to be available inside res.headers, instead only one is available 2) Response JSON shown in devtools console shows that no Origin header was among request headers 3) Code from entrypoint.js outputs 2 response headers when it's run from browser console (Origin header is sent)

deepak1556 commented 8 years ago

Thanks for the test case, the problem seems to be with file scheme not sending origin: null in Electron. You can workaround it with a http(s) scheme.

1999 commented 8 years ago

You can workaround it with a http(s) scheme.

eh... can you tell a bit more about it? What should I do to let my client-side code send Origin header?

deepak1556 commented 8 years ago

@zcbenz the cause seems to be enabling allow_universal_access_from_file_urls which made the request to be not treated as CORS, can we add a webPreferences option to disable it ? This will only affect whether JavaScript running in the context of a file scheme can access content from any origin. If we were to disable it explicitly, it will affect users of custom protocol who might not have CORS enabled.

@1999 i meant serving the file from a local http server instead of loading from file scheme.

1999 commented 8 years ago

That'a a bit sad because I want my app to be fully working offline. Is there any other workaround for this issue? UPD: I've just tried fs.readFileSync + new Blob + URL.createObjectURL + appending script to body, but blob URLs also have this problem.

zcbenz commented 8 years ago

This seems to be caused by the origin header not being set in file: scheme? Using custom protocols should be able to solve this: http://electron.atom.io/docs/api/protocol/.

deepak1556 commented 8 years ago

This seems to be caused by the origin header not being set in file: scheme?

Yup, that's because we have enabled the Content::WebPreferences option allow_universal_access_from_file_urls which makes the request to any origin from file scheme not to be treated as CORS. So was thinking if we can provide an option to disable it for users who require the default browser behavior.

deepak1556 commented 8 years ago

Using custom protocols should be able to solve this

Yup custom protocols can be used too, in that case we can simply document this behavior for file scheme ?

zcbenz commented 8 years ago

Adding an option for allow_universal_access_from_file_urls is fine, but disabling an feature to enable another one is usually not an option, because users may want both.

zcbenz commented 8 years ago

Yup custom protocols can be used too, in that case we can simply document this behavior for file scheme ?

Yeah it sounds good to me, we can probably have a page listing all limitations of file scheme and recommend using custom protocols instead.

I'm closing this since this is a limitation of file scheme and customs protocols should be used in this case.

1999 commented 8 years ago

Thank you guys!

1999 commented 8 years ago

Btw, which of the "registerSmthProtocol" methods should I use? When I use registerFileProtocol or registerStringProtocol, Origin header is not sent. registerHttpProtocol requires sending redirectRequest as the output, but I want my app to be available offline (at least have a small working set of features).

deepak1556 commented 8 years ago

@1999 any custom protocol should support CORS, i tested your example with custom scheme and origin headers are sent.

'use strict';

const {app, BrowserWindow, session} = require('electron');
let win;

function createWindow () {
  win = new BrowserWindow({
    width: 800,
    height: 600
  });

  session.defaultSession.protocol.registerFileProtocol('app', (request, callback) => {
    callback(__dirname + '/index.html')
  })

  win.loadURL('app://host')
  win.on('closed', () => {
    win = null
  })
}

app.on('ready', createWindow)
<script>
'use strict';

const url = 'https://pokormikota.ru/_cors';
const headers = new Headers;

fetch(url, {headers})
    .then(res => {
        return res.json();
    })
    .then(json => console.log(json.request_headers))
    .catch(console.error.bind(console));
</script>

output:

{
  "host": "pokormikota.ru",
  "x-real-ip": "49.206.116.5",
  "x-forwarded-for": "49.206.116.5",
  "x-forwarded-proto": "https",
  "connection": "close",
  "origin": "app://",
  "user-agent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) PepperMemory/0.0.1 Chrome/52.0.2743.82 Electron/1.3.5 Safari/537.36",
  "accept": "*/*",
  "accept-encoding": "gzip, deflate",
  "accept-language": "en-US",
  "if-none-match": "W/\"1b6-1bc27e50\""
}