DSN::to_string() produces invalid DSN string

[JonasKruckenberg]

Environment

sentry-rust => "0.27.0"
@sentry/browser => ^7.2.0"

Steps to Reproduce

1. Parse DSN from string
2. call .to_string() on dsn
3. parse dsn string again to verify parsing fails (in my case this was parsed by the JS SDK, but that should not matter)

Expected Result

string representation:
"https://db7855f82cec4baca8a0d6ec8d8f5d88@o4503930427473920.ingest.sentry.io/4503931193851907"

Actual Result

string representation
"https://db7855f82cec4baca8a0d6ec8d8f5d88:@o4503930427473920.ingest.sentry.io/4503931193851907"
notice the superfluous colon.

[Swatinem]

I agree this looks a bit weird, but having an empty password portion is perfectly valid.

There is a bunch of tests here that verify this works, at least on the Rust side:

sentry-rust/sentry-types/src/dsn.rs

Lines 221 to 318 in a433f63

	#[test]
	fn test_dsn_serialize_deserialize() {
	let dsn = Dsn::from_str("https://username:@domain/42").unwrap();
	let serialized = serde_json::to_string(&dsn).unwrap();
	assert_eq!(serialized, "\"https://username:@domain/42\"");
	let deserialized: Dsn = serde_json::from_str(&serialized).unwrap();
	assert_eq!(deserialized.to_string(), "https://username:@domain/42");
	}
	#[test]
	fn test_dsn_parsing() {
	let url = "https://username:password@domain:8888/23%21";
	let dsn = url.parse::<Dsn>().unwrap();
	assert_eq!(dsn.scheme(), Scheme::Https);
	assert_eq!(dsn.public_key(), "username");
	assert_eq!(dsn.secret_key(), Some("password"));
	assert_eq!(dsn.host(), "domain");
	assert_eq!(dsn.port(), 8888);
	assert_eq!(dsn.path(), "/");
	assert_eq!(dsn.project_id(), &ProjectId::new("23%21"));
	assert_eq!(url, dsn.to_string());
	}
	#[test]
	fn test_dsn_no_port() {
	let url = "https://username:@domain/42";
	let dsn = Dsn::from_str(url).unwrap();
	assert_eq!(dsn.port(), 443);
	assert_eq!(url, dsn.to_string());
	assert_eq!(
	dsn.store_api_url().to_string(),
	"https://domain/api/42/store/"
	);
	assert_eq!(
	dsn.envelope_api_url().to_string(),
	"https://domain/api/42/envelope/"
	);
	}
	#[test]
	fn test_insecure_dsn_no_port() {
	let url = "http://username:@domain/42";
	let dsn = Dsn::from_str(url).unwrap();
	assert_eq!(dsn.port(), 80);
	assert_eq!(url, dsn.to_string());
	assert_eq!(
	dsn.store_api_url().to_string(),
	"http://domain/api/42/store/"
	);
	assert_eq!(
	dsn.envelope_api_url().to_string(),
	"http://domain/api/42/envelope/"
	);
	}
	#[test]
	fn test_dsn_no_password() {
	let url = "https://username:@domain:8888/42";
	let dsn = Dsn::from_str(url).unwrap();
	assert_eq!(url, dsn.to_string());
	assert_eq!(
	dsn.store_api_url().to_string(),
	"https://domain:8888/api/42/store/"
	);
	assert_eq!(
	dsn.envelope_api_url().to_string(),
	"https://domain:8888/api/42/envelope/"
	);
	}
	#[test]
	fn test_dsn_no_password_colon() {
	let url = "https://username@domain:8888/42";
	let dsn = Dsn::from_str(url).unwrap();
	assert_eq!("https://username:@domain:8888/42", dsn.to_string());
	}
	#[test]
	fn test_dsn_http_url() {
	let url = "http://username:@domain:8888/42";
	let dsn = Dsn::from_str(url).unwrap();
	assert_eq!(url, dsn.to_string());
	}
	#[test]
	fn test_dsn_non_integer_project_id() {
	let url = "https://username:password@domain:8888/abc123youandme%21%21";
	let dsn = url.parse::<Dsn>().unwrap();
	assert_eq!(dsn.project_id(), &ProjectId::new("abc123youandme%21%21"));
	}
	#[test]
	fn test_dsn_more_than_one_non_integer_path() {
	let url = "http://username:@domain:8888/pathone/pathtwo/pid";
	let dsn = url.parse::<Dsn>().unwrap();
	assert_eq!(dsn.project_id(), &ProjectId::new("pid"));
	assert_eq!(dsn.path(), "/pathone/pathtwo/");
	}

I believe this is rather a problem in the JS SDK then when it rejects such URL patterns.

But I do agree that having the username/password separator without an actual password looks out of place, and might be worth changing.

[Swatinem]

https://www.ietf.org/rfc/rfc1738.txt section 3.1 talks about this, and username with empty password is valid according to that RFC.

Can you elaborate which API you are using to parse things that is failing?

[AbhiPrasad]

We also should be parsing this properly in the JS SDK as well: https://github.com/getsentry/sentry-javascript/blob/master/packages/utils/test/dsn.test.ts, we can add more test cases to validate though if you can let us know the steps you are taking @JonasKruckenberg

[JonasKruckenberg]

Alright, thanks for your fast response 😉

So TLDR; the JS SDK rejects that DSN string when the password is empty. I tracked this down to this line
https://github.com/getsentry/sentry-javascript/blob/8e967c339903a2bb6328f1f20c4a50712f78ed17/packages/utils/src/dsn.ts#L39 where the regex fails to account for empty password strings.

Adding a star after the password capture group should fix this issue (here is playground with the fix)

- const DSN_REGEX = /^(?:(\w+):)\/\/(?:(\w+)(?::(\w+))?@)([\w.-]+)(?::(\d+))?\/(.+)/;
+ const DSN_REGEX = /^(?:(\w+):)\/\/(?:(\w+)(?::(\w+)*)?@)([\w.-]+)(?::(\d+))?\/(.+)/;

For reference this the full process that I am doing:

I have a Rust backend and a JS frontend (a Tauri app) and I want to avoid people having to configure their DSN in two different places, so I have my Rust backend parse the DSN like so:

let sentry_options = sentry::ClientOptions {
    dsn:
      "https://db7855f82cec4baca8a0d6ec8d8f5d88@o4503930427473920.ingest.sentry.io/4503931193851907"
        .into_dsn()
        .expect("failed to parse DSN"),
    release: sentry::release_name!(),
    ..Default::default()
  };

and then later down the line inject that DSN into my javascript like so:

let js_init_script = format!(
    "window.__SENTRY_DSN__ = JSON.parse({:?})", dsn.expect("A DSN must be configured").to_string()
  );

the JS sentry SDK is then simply initialised the same way the getting started guide says, with the only exception that the DSN is taken from that window global:

Sentry.init({
    dsn: window.__SENTRY_DSN__
  });

that last code snippet errors out when the DSN string contains that empty password.

[JonasKruckenberg]

I made a quick PR for this now that I basically figured out the solution anyway getsentry/sentry-javascript#5902 😄

[AbhiPrasad]

@timfish might be interested in what you are doing with Tauri! He did some work with https://github.com/timfish/sentry-tauri

[JonasKruckenberg]

@timfish might be interested in what you are doing with Tauri! He did some work with https://github.com/timfish/sentry-tauri

Yeah my plugin is actually a fork of his that improves the DX somewhat 😄
I was actually just wondering if you people would be interested in collaborating with us (the Tauri team) on building a more official integration for tauri (though I probably have to ask someone else for this right? 😅) I've been using Sentry for a couple days now and am generally very pleased! Just a few things that could be improved with Sentry actually knowing about Tauri (for example the platform is currently reported as "Apple Mail")

[AbhiPrasad]

@JonasKruckenberg sounds good! Mind reaching out on Discord? Probably easier to get other folks involved that way and have a shorter response back and forth.

[Swatinem]

Ah nice, so this turned into an improvement of the JS SDK.

I will close this issue then, since there seems to be nothing wrong with to_string from the Rust side.