Skip to content

fix: apply withCredentials for sync requests (#168)#195

Open
Hoang130203 wants to merge 1 commit intonaugtur:masterfrom
Hoang130203:fix/withCredentials-sync-requests
Open

fix: apply withCredentials for sync requests (#168)#195
Hoang130203 wants to merge 1 commit intonaugtur:masterfrom
Hoang130203:fix/withCredentials-sync-requests

Conversation

@Hoang130203
Copy link
Copy Markdown

Problem

Fixes #168

withCredentials was silently ignored for sync requests. The option was set inside an if (!sync) guard:

xhr.open(method, uri, !sync, options.username, options.password)
//has to be after open
if(!sync) {
    xhr.withCredentials = !!options.withCredentials  // ← sync requests never reach this
}

This guard was introduced to work around a Firefox crash when withCredentials was always forced true on sync requests — regardless of what the user passed.

Root cause distinction

The original crash was caused by unconditionally setting withCredentials = true on sync requests. The fix over-corrected by removing it entirely for sync.

The real safe behaviour is: only set withCredentials when the caller explicitly opts in. Setting xhr.withCredentials = false (the default when options.withCredentials is falsy) on a sync request does not trigger the Firefox crash, and setting it true only when the user asks for it is safe in all modern browsers.

Fix

Remove the if (!sync) guard so withCredentials is applied consistently for both sync and async requests:

xhr.open(method, uri, !sync, options.username, options.password)
//has to be after open
xhr.withCredentials = !!options.withCredentials  // applied for all requests

This is consistent with how jQuery handles it and matches the behaviour a user would reasonably expect.

)

Previously withCredentials was guarded inside `if(!sync)`, meaning sync
requests silently ignored the withCredentials option entirely.

The guard was introduced to work around a Firefox crash when
withCredentials=true was *always* forced on sync requests. The fix here
is to only set withCredentials when the caller explicitly requests it
(i.e. options.withCredentials is truthy), which is safe for both sync
and async in all modern browsers.

Before:
  xhr.open(method, uri, !sync, ...)
  if (!sync) {
    xhr.withCredentials = !!options.withCredentials  // sync silently ignored
  }

After:
  xhr.open(method, uri, !sync, ...)
  xhr.withCredentials = !!options.withCredentials    // applied for all requests
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.

withCredentials not enabled for sync requests

1 participant