1346747 - No error when preflight results in 100

Status: RESOLVED INVALID

(Core :: DOM: Core & HTML, enhancement, P2)

Description

[Anne (:annevk)]

web-platform-tests:

  cors/preflight-failure.htm

Comment 1

[Andrew Overholt [:overholt]]

Jason, does your team know more about this kind of thing or is it more bz territory?

Comment 2

[Jason Duell]

Sadly we don't have a lot of knowledge about CORS details. I used to ask :sicking.  Now I'd think someone in sec-eng?

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

ckerschb, could you take a look? If not, I'll try to find time.

Comment 4

[Christoph Kerschbaumer [:ckerschb}]

(In reply to Anne (:annevk) from comment #0)
> web-platform-tests:
> 
>   cors/preflight-failure.htm

Anne, I was about to look into this problem, but it seems I can't find that test in our codebase. Shouldn't it be within 'testing/web-platform/tests/cors'? Please let me know where to find the test and I'll have a look.

Comment 5

[Anne (:annevk)]

http://w3c-test.org/cors/preflight-failure.htm has a copy. https://w3c-test.org/cors/preflight-failure.htm does too (with slightly different failures; I'm guessing it only works for HTTP, haven't looked closely).

(This also means we synchronize with web-platform-tests less regularly than I thought.)

Comment 6

[Christoph Kerschbaumer [:ckerschb}]

Thn(In reply to Anne (:annevk) from comment #5)
> http://w3c-test.org/cors/preflight-failure.htm has a copy.
> https://w3c-test.org/cors/preflight-failure.htm does too (with slightly
> different failures; I'm guessing it only works for HTTP, haven't looked
> closely).

Thanks Anne. Looking at this I can't really figure out where the problem happens. Probably we do something funky for http responses of 1**. Interesting observation is that within Chrome and Safari not only the 100 test fails but also the 101.

Valentin, you know Necko code way better than I do, it seems we are not calling nsCORSListenerProxy::CheckRequestApproved() for the 100 testcase which would then cause the error to happen. E.g. looking at stacktrace underneath for the 101 testcase (which works in Firefox), anything that pops into your mind where the code might diverge between a http status code of 100 vs 101?


0  0x00007f58a8eae75d in nanosleep () at ../sysdeps/unix/syscall-template.S:84
#1  0x00007f58a8eae6aa in __sleep (seconds=0) at ../sysdeps/posix/sleep.c:55
#2  0x00007f589adb6e5f in ah_crap_handler (signum=11) at /home/ckerschb/source/mc/toolkit/xre/nsSigHandlers.cpp:103
#3  0x00007f589ad94c56 in nsProfileLock::FatalSignalHandler (signo=11, info=0x7ffefcc1c4b0, context=0x7ffefcc1c380)
    at /home/ckerschb/source/mc/toolkit/profile/nsProfileLock.cpp:191
#4  0x00007f589b7eb992 in js::UnixExceptionHandler (signum=11, info=0x7ffefcc1c4b0, context=0x7ffefcc1c380)
    at /home/ckerschb/source/mc/js/src/ds/MemoryProtectionExceptionHandler.cpp:263
#5  0x00007f589bc4962f in WasmFaultHandler<(Signal)0> (signum=11, info=0x7ffefcc1c4b0, context=0x7ffefcc1c380)
    at /home/ckerschb/source/mc/js/src/wasm/WasmSignalHandlers.cpp:1246
#6  <signal handler called>
#7  0x00007f589579c185 in nsCORSListenerProxy::CheckRequestApproved (this=0x7f587c278220, aRequest=0x7f587bb97858)
    at /home/ckerschb/source/mc/netwerk/protocol/http/nsCORSListenerProxy.cpp:548
#8  0x00007f589579ba9a in nsCORSListenerProxy::OnStartRequest (this=0x7f587c278220, aRequest=0x7f587bb97858, aContext=0x0)
    at /home/ckerschb/source/mc/netwerk/protocol/http/nsCORSListenerProxy.cpp:466
#9  0x00007f5895606445 in nsUnknownDecoder::FireListenerNotifications (this=0x7f587b756820, request=0x7f587bb97858, aCtxt=0x0)
    at /home/ckerschb/source/mc/netwerk/streamconv/converters/nsUnknownDecoder.cpp:678
#10 0x00007f58956048bf in nsUnknownDecoder::OnStopRequest (this=0x7f587b756820, request=0x7f587bb97858, aCtxt=0x0, aStatus=nsresult::NS_OK)
    at /home/ckerschb/source/mc/netwerk/streamconv/converters/nsUnknownDecoder.cpp:289
#11 0x00007f58957c6903 in mozilla::net::nsHttpChannel::OnStopRequest (this=0x7f587bb97800, request=0x7f58862b9480, ctxt=0x0, status=nsresult::NS_OK)
    at /home/ckerschb/source/mc/netwerk/protocol/http/nsHttpChannel.cpp:7093
#12 0x00007f58952f66f6 in nsInputStreamPump::OnStateStop (this=0x7f58862b9480) at /home/ckerschb/source/mc/netwerk/base/nsInputStreamPump.cpp:715
#13 0x00007f58952f5743 in nsInputStreamPump::OnInputStreamReady (this=0x7f58862b9480, stream=0x7f587ccace90)
    at /home/ckerschb/source/mc/netwerk/base/nsInputStreamPump.cpp:433
#14 0x00007f589518831b in nsInputStreamReadyEvent::Run (this=0x7f58862146a0) at /home/ckerschb/source/mc/xpcom/io/nsStreamUtils.cpp:96
#15 0x00007f58951d635a in nsThread::ProcessNextEvent (this=0x7f5892285e80, aMayWait=false, aResult=0x7ffefcc1cf67)
    at /home/ckerschb/source/mc/xpcom/threads/nsThread.cpp:1269
#16 0x00007f58951db825 in NS_ProcessNextEvent (aThread=0x7f5892285e80, aMayWait=false) at /home/ckerschb/source/mc/xpcom/threads/nsThreadUtils.cpp:389
#17 0x00007f58959f59a2 in mozilla::ipc::MessagePump::Run (this=0x7f58922bb300, aDelegate=0x7f58a8b640b0) at /home/ckerschb/source/mc/ipc/glue/MessagePump.cpp:96
#18 0x00007f5895948914 in MessageLoop::RunInternal (this=0x7f58a8b640b0) at /home/ckerschb/source/mc/ipc/chromium/src/base/message_loop.cc:238
#19 0x00007f5895948898 in MessageLoop::RunHandler (this=0x7f58a8b640b0) at /home/ckerschb/source/mc/ipc/chromium/src/base/message_loop.cc:231
#20 0x00007f589594885c in MessageLoop::Run (this=0x7f58a8b640b0) at /home/ckerschb/source/mc/ipc/chromium/src/base/message_loop.cc:211
#21 0x00007f5898c16ffe in nsBaseAppShell::Run (this=0x7f5888f43a20) at /home/ckerschb/source/mc/widget/nsBaseAppShell.cpp:156
#22 0x00007f589aca1ce7 in nsAppStartup::Run (this=0x7f5888f241f0) at /home/ckerschb/source/mc/toolkit/components/startup/nsAppStartup.cpp:283
#23 0x00007f589adab6d8 in XREMain::XRE_mainRun (this=0x7ffefcc1d3b0) at /home/ckerschb/source/mc/toolkit/xre/nsAppRunner.cpp:4520
#24 0x00007f589adac19c in XREMain::XRE_main (this=0x7ffefcc1d3b0, argc=4, argv=0x7ffefcc1e748, aConfig=...)
    at /home/ckerschb/source/mc/toolkit/xre/nsAppRunner.cpp:4700
#25 0x00007f589adac4a1 in XRE_main (argc=4, argv=0x7ffefcc1e748, aConfig=...) at /home/ckerschb/source/mc/toolkit/xre/nsAppRunner.cpp:4791
#26 0x00007f589adc091a in mozilla::BootstrapImpl::XRE_main (this=0x7f58a8bb70a8, argc=4, argv=0x7ffefcc1e748, aConfig=...)
    at /home/ckerschb/source/mc/toolkit/xre/Bootstrap.cpp:45
#27 0x00000000004062a7 in do_main (argc=4, argv=0x7ffefcc1e748, envp=0x7ffefcc1e770) at /home/ckerschb/source/mc/browser/app/nsBrowserApp.cpp:236
#28 0x0000000000406534 in main (argc=4, argv=0x7ffefcc1e748, envp=0x7ffefcc1e770) at /home/ckerschb/source/mc/browser/app/nsBrowserApp.cpp:307

Comment 7

[Valentin Gosu [:valentin] {{ PTO 2 - 12 April }}]

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #6)
> Valentin, you know Necko code way better than I do, it seems we are not
> calling nsCORSListenerProxy::CheckRequestApproved() for the 100 testcase
> which would then cause the error to happen. E.g. looking at stacktrace
> underneath for the 101 testcase (which works in Firefox), anything that pops
> into your mind where the code might diverge between a http status code of
> 100 vs 101?

The only difference I can see is that we call CompleteUpgrade for 101 status codes in nsHttpChannel::OnStopRequest

Comment 8

[Christoph Kerschbaumer [:ckerschb}]

Anne, is it possible that the test is phishy given that Chrome, Firefox, Safari all fail that test?

Comment 9

[Anne (:annevk)]

Indeed, it's probably not possible to test those status codes reliably with the current infrastructure. Filed https://github.com/w3c/web-platform-tests/pull/5378 to get the test changed (review appreciated) and filed https://github.com/w3c/wptserve/issues/117 to get better infrastructure.