Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

$http fails on LG webOS using the file:// protocol as cookies aren't accessible #15523

Closed
sp00m opened this issue Dec 19, 2016 · 4 comments
Closed

Comments

@sp00m
Copy link
Contributor

sp00m commented Dec 19, 2016

Note: for support questions, please use one of these channels: https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#question. This repository's issues are reserved for feature requests and bug reports.

Do you want to request a feature or report a bug?

bug

What is the current behavior?

On LG webOS using the file:// protocol, $http fails because it calls $$cookieReader that throws SecurityError: DOM Exception 18.

$http calls $$cookieReader for CSRF checks: https://github.com/angular/angular.js/blob/v1.6.0/src/ng/http.js#L1287.

$$cookieReader throws the error when accessing rawDocument.cookie: https://github.com/angular/angular.js/blob/v1.6.0/src/ng/cookieReader.js#L27

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://plnkr.co or similar (template: http://plnkr.co/edit/tpl:yBpEi4).

Quite hard to reproduce, as you need an LG webOS device... Just make $$cookieReader throw an error?

What is the expected behavior?

Either $http or $$cookieReader could catch errors so that xsrfValue gets undefined if cookies aren't accessible?

What is the motivation / use case for changing the behavior?

To fix a bug :)

Which versions of Angular, and which browser / OS are affected by this issue? Did this work in previous versions of Angular? Please also test with the latest stable and snapshot (https://code.angularjs.org/snapshot/) versions.

Issue met with 1.6.0 on LG webOS.

Other information (e.g. stacktraces, related issues, suggestions how to fix)

Actual stack trace:

DOMException
  code: 18
  column: 42
  constructor: DOMExceptionConstructor
  line: 20114
  message: "SecurityError: DOM Exception 18"
  name: "SecurityError"
  sourceURL: "file:///.../libs/angular/angular.js"
  stack: "
    file:///.../libs/angular/angular.js:20114:42
    sendReq@file:///.../libs/angular/angular.js:12146:29
    serverRequest@file:///.../libs/angular/angular.js:11905:23
    processQueue@file:///.../libs/angular/angular.js:16643:39
    file:///.../libs/angular/angular.js:16683:39
    $eval@file:///.../libs/angular/angular.js:17958:28
    $digest@file:///.../libs/angular/angular.js:17772:36
    $apply@file:///.../libs/angular/angular.js:18066:31
    bootstrapApply@file:///.../libs/angular/angular.js:1841:21
    invoke@file:///.../libs/angular/angular.js:4839:24
    doBootstrap@file:///.../libs/angular/angular.js:1839:20
    bootstrap@file:///.../libs/angular/angular.js:1859:23
    angularInit@file:///.../libs/angular/angular.js:1744:14
    file:///.../libs/angular/angular.js:32890:16
    mightThrow@file:///.../libs/jquery/dist/jquery.js:3570:34
    file:///.../libs/jquery/dist/jquery.js:3638:22
  "
  __proto__: DOMExceptionPrototype

For now, I temporary decorate $$cookieReader to disable it:

.config(["$provide",
function ($provide) {
  $provide.decorator("$$cookieReader", [
  function () {
    return function () {
      return {};
    };
  }]);
}])
@gkalpak
Copy link
Member

gkalpak commented Dec 19, 2016

This is a quite uncommon usecase (and has an easy - even if not ideal - workaround), but if you feel like it you can submit a PR fixing it. Basically, we could replace this line with something like:

-var currentCookieString = rawDocument.cookie || '';
+var currentCookieString = safeGetCookie(rawDocument);
+...
+function safeGetCookie(document) {
+  try {
+    return document.cookie || '';
+  } catch (err) {
+    return '';
+  }

@sp00m
Copy link
Contributor Author

sp00m commented Dec 20, 2016

Quite hard to test though... I've tried to play with a getter:

it('should return an empty string if cookie cannot be read', function() {
  // see #15523 - sometimes reading cookies throws an error
  document.cookie = 'cookie_name=cookie_value';
  Object.defineProperty(document, 'cookie', {
    configurable: true,
    get: function() { throw new Error('#15523'); }
  });
  expect($$cookieReader()['cookie_name']).toBeUndefined();
  Object.defineProperty(document, 'cookie', {
    writable: true
  });
});

But this breaks many other tests. Works as expected on this fiddle though: https://jsfiddle.net/31k61c3L/.

Any idea? Maybe I simply can't brutalise window.document like that...

@gkalpak
Copy link
Member

gkalpak commented Dec 20, 2016

Maybe I simply can't brutalise window.document like that...

No you can't (or rther you shouldn't). That's where DI comes into play 😃
$$cookieReader uses the injected $document to retrieve the rawDocument (as $document[0]). All you need to do, is provide a mock $document for your test, such that $document[0].cookie throws.

I haven't looked at the testsuite, but something along the following lines should work:

it('should return an empty string if cookie cannot be read', function() {
  var cookieSpy = jasmine.createSpy('cookie').and.throwError('Can\'t touch this!');
  var mockDocument = {};
  Object.defineProperty(mockDocument, 'cookie', {get: cookieSpy});

  module(function($provide) {
    $provide.value($document, [mockDocument]);
  });

  inject(function($$cookieReader) {
    expect($$cookieReader()).toEqual({});
    expect(cookieSpy).toHaveBeenCalled();
  });
});

EDIT:
I just saw how the $$cookieReader testsuite is set up. You need to have a separate describe block that the existing beforeEach/afterEach do not affect. (Let me know if you need help with that.)

@sp00m
Copy link
Contributor Author

sp00m commented Dec 20, 2016

Thanks @gkalpak, very educative!

gkalpak pushed a commit that referenced this issue Dec 20, 2016
…ookie`

In certain cases (e.g. on LG webOS using the `file:` protocol), access to
`document.cookie` may not be allowed and throw an error. This could break
`$http` which relies on `$$cookieReader()` for retrieving the XSRF token.
This commit fixes it by treating `document.cookie` as empty, when access to it
is fordibben.

Fixes  #15523

Closes #15532
gkalpak pushed a commit that referenced this issue Dec 20, 2016
…ookie`

In certain cases (e.g. on LG webOS using the `file:` protocol), access to
`document.cookie` may not be allowed and throw an error. This could break
`$http` which relies on `$$cookieReader()` for retrieving the XSRF token.
This commit fixes it by treating `document.cookie` as empty, when access to it
is fordibben.

Fixes  #15523

Closes #15532
ellimist pushed a commit to ellimist/angular.js that referenced this issue Mar 15, 2017
…ookie`

In certain cases (e.g. on LG webOS using the `file:` protocol), access to
`document.cookie` may not be allowed and throw an error. This could break
`$http` which relies on `$$cookieReader()` for retrieving the XSRF token.
This commit fixes it by treating `document.cookie` as empty, when access to it
is fordibben.

Fixes  angular#15523

Closes angular#15532
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants