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

Adds CORS support (XDomainRequest) for IE7+ still through XMLHttpRequest #107

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

3F
Copy link

@3F 3F commented Jan 8, 2019

Fixes #106

I think onreadystatechange is more than enough for our case. Otherwise, full support requires both XDomainRequest and XMLHttpRequest <_<

src/index.mjs Outdated
request.onload = () => {
resolve(response());
request.onreadystatechange = function() {
if(this.readyState === 4 && this.status === 200) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this.status check here should be removed. we only care that readyState is 4.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whoops, I leaved this from my described hack in #106 :)

For my personal task, readystate = 4 will be also triggered on CORS errors and for abort operations.
that is status = 0 for readystate = 4

wait

@3F
Copy link
Author

3F commented Jan 12, 2019

@developit

Let us return to our muttons :)

this.status check here should be removed. we only care that readyState is 4.

I was still worried about your statement about removing status = 200 from this PR. So...

Today I've checked implementation of XHR, specially to check this moment because I was not sure about fully equivalent to load events for just readystate = 4 (I just don't remember this moment, and can't find any official documentation about this today).

However, seems my first thought that appeared from my example #106 (well, exactly from my mentioned task), was initially a correct thought.

So I'm not agree with you :)

I'll illustrate possible bug via Firefox:

We will initialize onreadystatechange via FireReadystatechangeEvent() method

  • init -> adding for listeners -> then dispatching methods ->

basically the only readystatechange is a core event, while the others implementation just will be used when processing the main

Before final processing I'm also seeing decoding response and finally changing state and dispatching our additional events

there are many places that we will raise after, but mainly, this is what I'm talking about:

XMLHttpRequestMainThread::ChangeStateToDone()
{
...
if (mErrorLoad != ErrorType::eOK) {
  DispatchProgressEvent(this, ProgressEventType::error, 0, -1);
} else {
  DispatchProgressEvent(this, ProgressEventType::load,
                        mLoadTransferred, mLoadTotal);
}

when, for example:

void
XMLHttpRequestMainThread::CloseRequest()
{
  mWaitingForOnStopRequest = false;
  mErrorLoad = ErrorType::eTerminated;
  if (mChannel) {
    mChannel->Cancel(NS_BINDING_ABORTED);
  }

when, for example:

void
XMLHttpRequestMainThread::TerminateOngoingFetch() {
  if ((mState == XMLHttpRequest_Binding::OPENED && mFlagSend) ||
      mState == XMLHttpRequest_Binding::HEADERS_RECEIVED ||
      mState == XMLHttpRequest_Binding::LOADING) {
    CloseRequest();
  }
}

XMLHttpRequestMainThread.cpp

and so on ...

Thus! we will never have processing for onload events (javascript) when processing the same failed state for onreadystatechange (javascript)

That is, as I understand from this code, the onload event is equal to onreadystatechange only when

readyState === 4 && status !== 0

Or more probably(I've not inspected a lot), I'm seeing many places for changing status, like status from http channel via all overrided virtual methods

MOZ_MUST_USE NS_IMETHOD GetResponseStatus(uint32_t *aResponseStatus) = 0;

and other, but seems status === 200 (as it was initially from my example) is a correct way.

Unfortunately, I still can't find official documentation for compare logic onload vs onreadystatechange but from source code, it looks like this is it.

Please check this out, again.

I'll try also to prepare a common example later if needed (javascript of course) to reproduce this behavior.

@3F
Copy link
Author

3F commented Jan 12, 2019

I'm still here, and this is my explanation via javascript now:

let xhr = new XMLHttpRequest();

window.XMLHttpRequest.prototype.xD = function(evt)
{
    console.log(evt, this.readyState, this.status);
}

xhr.onreadystatechange = function()
{
    if(this.readyState !== 4) return;
    xhr.xD('readystatechange()');
}

xhr.onload = () => xhr.xD('load()');
xhr.onabort = () => xhr.xD('abort()');

xhr.open('GET', 'https://api.github.com/users/3F/repos');
xhr.send();

// readystatechange() 4 200
// load() 4 200

setTimeout(() => xhr.abort(), 100);

// readystatechange() 4 0      <<<<<<<
// abort() 4 0

xhr.open('GET', 'https://api.github.com/users/3F/NULL');
xhr.send();

// readystatechange() 4 404
// load() 4 404

xhr.abort();

// readystatechange() 4 0      <<<<<<<
// abort() 4 0

So I'm thinking at least for non 0 state :p Or I need to inspect more via original implementation (C++ src see above). For chromium etc please check it yourself. I will not.

Any related official doc/RFC is really appreciated. Because it maybe just a bug of engines (I'm not sure because I have no info), like:

  // The ChangeState call above calls onreadystatechange handlers which
  // if they load a new url will cause XMLHttpRequestMainThread::Open to clear
  // the abort state bit. If this occurs we're not uninitialized (bug 361773).
  if (mFlagAborted) {
    ChangeState(XMLHttpRequest_Binding::UNSENT, false);  // IE seems to do it
  }

Copy link
Author

@3F 3F left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fully reviewed an status code to get load event equivalent.

In addition to my explanation in #107, here's also testing of states for different http codes together with an abort operations:

...
let sleep = (ms) => {
    return new Promise(_ => setTimeout(_, ms))
}

let vrf = async(code, abort = false, srvsleep = 1000) =>
{
    console.log('==== ' + code + ' : abort = ' + abort);
    
    xhr.open('GET', 'https://httpstat.us/' + code + '?sleep=' + srvsleep, true);
    xhr.send();
    
    if(abort)
    {
        await sleep(500);
        xhr.abort();
        return;
    }

    let attempt = 0;
    while(xhr.readyState !== 4)
    {
        if(++attempt > 20) {
            console.log('Stop waiting an readyState. /' + xhr.readyState);
            return;
        }
        await sleep(250);
    }
}

let xhrTest = async (errors, delay = 1000) =>
{
    for(let err of errors)
    {
        await sleep(delay);
        await vrf(err, false);

        await sleep(delay);
        await vrf(err, true);
    }
}

xhrTest([100, 101, 102, 200, 201, 202, 203, 204, 205, 206, 300, 301, 302, 303, 304, 305, 306, 307, 308, 400, 401, 402, 403, 404, 405, 406, 407, 408, 409, 410, 411, 412, 413, 414, 415, 416, 417, 418, 422, 428, 429, 431, 451, 500, 501, 502, 503, 504, 505, 511, 520, 522, 524, ]);

the result:

==== 100 : abort = false
Stop waiting an readyState. /1
==== 100 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 101 : abort = false
Stop waiting an readyState. /1
==== 101 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 102 : abort = false
Stop waiting an readyState. /1
==== 102 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 200 : abort = false
	 readystatechange() 4 200
	 load() 4 200
==== 200 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 201 : abort = false
	 readystatechange() 4 201
	 load() 4 201
==== 201 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 202 : abort = false
	 readystatechange() 4 202
	 load() 4 202
==== 202 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 203 : abort = false
	 readystatechange() 4 203
	 load() 4 203
==== 203 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 204 : abort = false
	 readystatechange() 4 204
	 load() 4 204
==== 204 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 205 : abort = false
	 readystatechange() 4 205
	 load() 4 205
==== 205 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 206 : abort = false
	 readystatechange() 4 206
	 load() 4 206
==== 206 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 300 : abort = false
	 readystatechange() 4 300
	 load() 4 300
==== 300 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 301 : abort = false
	 readystatechange() 4 200
	 load() 4 200
==== 301 : abort = true
	 readystatechange() 4 200
	 load() 4 200
==== 302 : abort = false
	 readystatechange() 4 200
	 load() 4 200
==== 302 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 303 : abort = false
	 readystatechange() 4 200
	 load() 4 200
==== 303 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 304 : abort = false
	 readystatechange() 4 304
	 load() 4 304
==== 304 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 305 : abort = false
	 readystatechange() 4 305
	 load() 4 305
==== 305 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 306 : abort = false
	 readystatechange() 4 306
	 load() 4 306
==== 306 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 307 : abort = false
	 readystatechange() 4 200
	 load() 4 200
==== 307 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 308 : abort = false
	 readystatechange() 4 200
	 load() 4 200
==== 308 : abort = true
	 readystatechange() 4 200
	 load() 4 200
==== 400 : abort = false
	 readystatechange() 4 400
	 load() 4 400
==== 400 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 401 : abort = false
	 readystatechange() 4 401
	 load() 4 401
==== 401 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 402 : abort = false
	 readystatechange() 4 402
	 load() 4 402
==== 402 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 403 : abort = false
	 readystatechange() 4 403
	 load() 4 403
==== 403 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 404 : abort = false
	 readystatechange() 4 404
	 load() 4 404
==== 404 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 405 : abort = false
	 readystatechange() 4 405
	 load() 4 405
==== 405 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 406 : abort = false
	 readystatechange() 4 406
	 load() 4 406
==== 406 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 407 : abort = false
	 readystatechange() 4 407
	 load() 4 407
==== 407 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 408 : abort = false
	 readystatechange() 4 408
	 load() 4 408
==== 408 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 409 : abort = false
	 readystatechange() 4 409
	 load() 4 409
==== 409 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 410 : abort = false
	 readystatechange() 4 410
	 load() 4 410
==== 410 : abort = true
	 readystatechange() 4 410
	 load() 4 410
==== 411 : abort = false
	 readystatechange() 4 411
	 load() 4 411
==== 411 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 412 : abort = false
	 readystatechange() 4 412
	 load() 4 412
==== 412 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 413 : abort = false
	 readystatechange() 4 413
	 load() 4 413
==== 413 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 414 : abort = false
	 readystatechange() 4 414
	 load() 4 414
==== 414 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 415 : abort = false
	 readystatechange() 4 415
	 load() 4 415
==== 415 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 416 : abort = false
	 readystatechange() 4 416
	 load() 4 416
==== 416 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 417 : abort = false
	 readystatechange() 4 417
	 load() 4 417
==== 417 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 418 : abort = false
	 readystatechange() 4 418
	 load() 4 418
==== 418 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 422 : abort = false
	 readystatechange() 4 422
	 load() 4 422
==== 422 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 428 : abort = false
	 readystatechange() 4 428
	 load() 4 428
==== 428 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 429 : abort = false
	 readystatechange() 4 429
	 load() 4 429
==== 429 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 431 : abort = false
	 readystatechange() 4 431
	 load() 4 431
==== 431 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 451 : abort = false
	 readystatechange() 4 451
	 load() 4 451
==== 451 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 500 : abort = false
	 readystatechange() 4 500
	 load() 4 500
==== 500 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 501 : abort = false
	 readystatechange() 4 501
	 load() 4 501
==== 501 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 502 : abort = false
	 readystatechange() 4 502
	 load() 4 502
==== 502 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 503 : abort = false
	 readystatechange() 4 503
	 load() 4 503
==== 503 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 504 : abort = false
	 readystatechange() 4 504
	 load() 4 504
==== 504 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 505 : abort = false
	 readystatechange() 4 505
	 load() 4 505
==== 505 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 511 : abort = false
	 readystatechange() 4 511
	 load() 4 511
==== 511 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 520 : abort = false
	 readystatechange() 4 520
	 load() 4 520
==== 520 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 522 : abort = false
	 readystatechange() 4 522
	 load() 4 522
==== 522 : abort = true
	 readystatechange() 4 0
	 abort() 4 0
==== 524 : abort = false
	 readystatechange() 4 524
	 load() 4 524
==== 524 : abort = true
	 readystatechange() 4 0
	 abort() 4 0 

It means that status shouldn't be equal at least to zero:

100_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
101_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
102_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
200_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
201_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
202_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
203_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
204_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
205_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
206_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
300_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
302_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
303_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
304_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
305_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
306_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
307_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
400_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
401_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
402_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
403_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
404_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
405_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
406_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
407_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
408_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
409_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
411_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
412_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
413_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
414_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
415_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
416_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
417_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
418_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
422_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
428_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
429_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
431_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
451_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
500_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
501_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
502_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
503_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
504_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
505_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
511_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
520_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
522_abort=true > { load: false, status: 0, abort: true, readystatechange: true }
524_abort=true > { load: false, status: 0, abort: true, readystatechange: true }

That is, we need to check it exactly as:

this.readyState === 4 && this.status !== 0

if the load event equivalent is needed, of course.

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

Successfully merging this pull request may close these issues.

None yet

2 participants