1002229 - Add anonymous parameter to Request API

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, enhancement, P2)

Description

[PerfectSlayer]

Hi,

I propose an update of the Request module.
I would like to add an anon parameter bound on the mozAnon parameter of the XMLHttpRequest object. It could be use to enable anonymous mode to prevent exposing user crendentials and sending cookies.

Thanks.

Comment 1

[PerfectSlayer]

Attachment: Pull request

Comment 2

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Is this something we want to do, Irakli, or should this just use xhr?

Comment 3

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

In SDK we do not usually abbreviate names. I'm ok with exposing this feature, but Bruce could you please rename option to `anonymous` instead ?

Thanks

Comment 4

[PerfectSlayer]

Absolutely !
I will rename the option to « anonymous » as proposed.

Comment 5

[Tomislav Jovanovic :zombie]

Comment on attachment 8413413 [details] [review]
Pull request

this looks good, but i'm gonna r- because i would like some functional tests.

you are changing how every XHR object is created (line 70), so i would like a test that ensures this doesn't break existing functionality, plus a test that this param actually works, and that XHR doesn't send (previously existing) cookies when anonymous is set.

i see there are other cookie-related tests, so i'm assuming this is possible with our current test harness. it could go something like this:

a) mock up (or find) a test-url that sends a cookie in a response
b) issue a first request that should receive (and store) the cookie
c) issue a second request and verify that XHR is sending the cookie
d) issue an anonymous third request and verify that XHR is not sending it

Comment 6

[PerfectSlayer]

I agree with the need of functional tests.

But the fact is I never succeed to pass any test from github master. I tried on my Win7 and Debian desktops (even on a Win8.1..) Never the cfx testxxx commands end.
I really would like to contribute but I already spent many hours without having a working test environment. And MDN and Google did not help me until now..

Comment 7

[Tomislav Jovanovic :zombie]

> I already spent many hours without having a working test environment. 

sorry to hear that. you should feel free to ask whenever you get stuck, or for a quicker response, you can join us in the #jetpack channel on irc.mozilla.org.


> Never the cfx testxxx commands end.

first of all, you should run tests with Nightly version of firefox, which is controlled with the cfx -b argument. second, you can limit to running only unit tests for the module you are changing if your changes are unlikely to break other parts of the SDK (which is the case here). and third, -v flag turns on verbose mode, which makes it easier to tell what's going on.. 

so try with this command line:

    cfx testpkgs -f request -v -b /path/to/nightly/firefox.exe


and again, thanks, and ask if i didn't explain something clearly! ;)

Comment 8

[PerfectSlayer]

A big thanks for your reply !

I fix remarks made on GitHub and add new functional tests using the scenario described above.
I hope it matches what you expected.

Comment 9

[Tomislav Jovanovic :zombie]

Comment on attachment 8413413 [details] [review]
Pull request

great work Bruce!  r+

thank you for this, and sorry for the delayed review.

i left some minor comments in the PR, it would be nice if you could address them (plus one left from the first review, about the mozAnon property).

after that, please squash all your commits into one (using rebase), we prefer to have PRs correspond to single commits, to make it easier to track them.

just post a comment here when you are done, and i'll merge it!

Comment 10

[PerfectSlayer]

I stuck about getting the mozAnon property from the XHR object.
See my reply about the inline comment (https://github.com/mozilla/addon-sdk/pull/1477#discussion_r13260959).

Otherwise, I fixed all other remarks and squashed my commits.

Comment 11

[PerfectSlayer]

The last remark was fixed. Thanks !

Feel free to merge it ;)

Comment 12

[[github robot]]

Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/d347cb5b97f54aae4e68c3ee6d718bf72f2a02b1
Merge pull request #1477 from PerfectSlayer/anon-request

Bug 1002229 - Add anon parameter to Request API, r=@zombie

Comment 13

[Tomislav Jovanovic :zombie]

good perseverance Bruce!  ;)

if you are interested in contributing some more, here is a list of other good starting bugs:

https://bugzilla.mozilla.org/buglist.cgi?status_whiteboard=[good+first+bug]&product=Add-on+SDK

Comment 14

[PerfectSlayer]

You're welcome ! My first contribution to a Mozilla project ;)
I went one step further =D (I only "use" SDK until now)

Thanks for the link. I will have a look at it if I found spare time.