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

Zulip URL encoding doesn't work for dots #712

Closed
camelid opened this issue Jul 18, 2020 · 3 comments · Fixed by #723
Closed

Zulip URL encoding doesn't work for dots #712

camelid opened this issue Jul 18, 2020 · 3 comments · Fixed by #723

Comments

@camelid
Copy link
Member

camelid commented Jul 18, 2020

Rustbot link

https://rust-lang.zulipchat.com/#narrow/stream/233931-xxx/topic/Use.20sane.20defaults.20in.20x.py.20compiler-team.23326

Actual link

https://rust-lang.zulipchat.com/#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Use.20sane.20defaults.20in.20x.2Epy.20compiler-team.23326

If we compare the Rustbot link (and elaborate the xxx to the full stream name) against the actual link, the only difference that I see is that the actual link encodes the . in x.py as .2E, whereas Rustbot encodes it as just ..

From what I see in the rest of the link, it looks like Zulip encodes special characters as a . and then the ASCII hex code. Because of that, it seems a . must be encoded as its hex code.

Cc #701 (comment)

Cc @Mark-Simulacrum

@camelid
Copy link
Member Author

camelid commented Jul 19, 2020

Relevant code?

triagebot/src/zulip.rs

Lines 337 to 359 in 823fd7f

// See
// https://github.com/zulip/zulip/blob/46247623fc279/zerver/lib/url_encoding.py#L9
// ALWAYS_SAFE without `.` from
// https://github.com/python/cpython/blob/113e2b0a07c/Lib/urllib/parse.py#L772-L775
//
// ALWAYS_SAFE doesn't contain `.` because Zulip actually encodes them to be able
// to use `.` instead of `%` in the encoded strings
const ALWAYS_SAFE: &str =
"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789_-~";
let mut encoded_topic = Vec::new();
for ch in topic.bytes() {
if !(ALWAYS_SAFE.contains(ch as char)) {
write!(encoded_topic, "%{:02X}", ch).unwrap();
} else {
encoded_topic.push(ch);
}
}
let mut encoded_topic = String::from_utf8(encoded_topic).unwrap();
encoded_topic = encoded_topic.replace("%", ".");
format!("stream/{}-xxx/topic/{}", id, encoded_topic)

@kellda
Copy link
Contributor

kellda commented Jul 22, 2020

Seems fixed since #653

@Mark-Simulacrum
Copy link
Member

I would like to add a test, then, just to make sure we don't regress this bit in the future.

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 a pull request may close this issue.

3 participants