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

Tracing headers are attached to protocol-relative URLs by default, causing CORS errors #8099

Closed
3 tasks done
gongpeione opened this issue May 11, 2023 · 2 comments · Fixed by #8114
Closed
3 tasks done

Comments

@gongpeione
Copy link

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/browser

SDK Version

7.48.0

Framework Version

No response

Link to Sentry event

No response

SDK Setup

const options = {
    ...
    tracesSampleRate: 1,
    integrations: [
        new Sentry.BrowserTracing()
    ]
}

Steps to Reproduce

  1. import and setup Sentry
  2. add a request like fetch('//xxx.xxx.com/xxx')
  3. run the code
  4. custom headers added to requests and causes CORS error

Expected Result

Based on my understanding of the line const DEFAULT_TRACE_PROPAGATION_TARGETS = ['localhost', /^\//]; Link, I would expect custom headers to only be added to non-cross-origin requests by default. This would be the case if the requests are targeting 'localhost' or if the request URL starts with a '/'.

However, I've noticed that this logic might not account for protocol-relative URLs (those starting with //, like //example.com). If my understanding is correct, this could potentially result in custom headers being added to these types of requests, which might not be the intended behavior

Actual Result

custom headers added to requests and cause CORS error.

If possible, I can submit a pull request to address this issue.

@Lms24
Copy link
Member

Lms24 commented May 11, 2023

Hi @gongpeione thanks for writing in!

Based on my understanding of the line const DEFAULT_TRACE_PROPAGATION_TARGETS = ['localhost', /^//]; Link, I would expect custom headers to only be added to non-cross-origin requests by default

Yes, that's exactly how the default value should behave.

However, I've noticed that this logic might not account for protocol-relative URLs (those starting with //, like //example.com).

Hmm you're right, this would match with the /^\// default regex. But to be honest, I didn't know that this is a valid and usable URL. Can you show us a concrete reproduction? Would fetch('//example.com') be one of them?

@gongpeione
Copy link
Author

Hi @gongpeione thanks for writing in!

Based on my understanding of the line const DEFAULT_TRACE_PROPAGATION_TARGETS = ['localhost', /^//]; Link, I would expect custom headers to only be added to non-cross-origin requests by default

Yes, that's exactly how the default value should behave.

However, I've noticed that this logic might not account for protocol-relative URLs (those starting with //, like //example.com).

Hmm you're right, this would match with the /^\// default regex. But to be honest, I didn't know that this is a valid and usable URL. Can you show us a concrete reproduction? Would fetch('//example.com') be one of them?

Yes, fetch('//example.com') could reproduce this situation.

image

@Lms24 Lms24 self-assigned this May 14, 2023
@Lms24 Lms24 changed the title sentry-trance and baggage header for request url like //xxx.com causes cors error sentry-trace and baggage header for request url like //xxx.com causes cors error May 15, 2023
@Lms24 Lms24 changed the title sentry-trace and baggage header for request url like //xxx.com causes cors error Tracing headers are attached to protocol-relative URLs by default, causing CORS errors May 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants