Skip to content

Percent-decode incoming queries#122

Merged
neumark merged 2 commits intomainfrom
percent-encode-query-special-chars
Sep 26, 2022
Merged

Percent-decode incoming queries#122
neumark merged 2 commits intomainfrom
percent-encode-query-special-chars

Conversation

@neumark
Copy link
Copy Markdown
Collaborator

@neumark neumark commented Sep 26, 2022

headers cannot contain special characters such as newlines (as is described in GH issue #104). The solution is to percent-encode (aka: encodeURIComponent()) the query string. Encoding is not mandatory, some characters like the space are HTTP header-safe, these characters can be sent both as they are or percent encoded (e.g. %20 for space).

@neumark neumark requested a review from mildbyte September 26, 2022 13:34
Comment thread Cargo.toml
log = "0.4"
moka = { version = "0.9.3", default_features = false, features = ["future", "atomic64", "quanta"] }
object_store = "0.5.0"
percent-encoding = "2.2.0"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, it also looks like we're already importing this as a transitive dependency (so it's not going to result in us pulling in extra code)

~/seafowl $ cargo tree | grep percent
│   │   │   │   ├── percent-encoding v2.2.0
│   │   │   │   │   ├── percent-encoding v2.2.0
│   │   │   │   │   │   │   └── percent-encoding v2.2.0
│   │   │   │   │   │   ├── percent-encoding v2.2.0
│   │   ├── percent-encoding v2.2.0
│           └── percent-encoding v2.2.0
│   ├── percent-encoding v2.2.0
    │   │   ├── percent-encoding v2.2.0

Comment thread src/frontend/http.rs Outdated
Comment on lines +223 to +229
let decoded_query = percent_decode_str(&raw_query).decode_utf8();

if decoded_query.is_err() {
return Err(ApiError::QueryDecodeError);
}

let hash_str = str_to_hex_hash(&decoded_query.as_ref().unwrap());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of using unwrap(), you should use the ? operator to bail out with an Err value and safely extract the Ok value otherwise:

Suggested change
let decoded_query = percent_decode_str(&raw_query).decode_utf8();
if decoded_query.is_err() {
return Err(ApiError::QueryDecodeError);
}
let hash_str = str_to_hex_hash(&decoded_query.as_ref().unwrap());
let decoded_query = percent_decode_str(&raw_query).decode_utf8().map_err(|_| ApiError::QueryDecodeError)?;
let hash_str = str_to_hex_hash(decoded_query);

(decoded_query should have the type String going ahead, so you don't need unwrap() which will panic at runtime if it encounters an Error value)

Comment thread src/frontend/http.rs Outdated
@neumark neumark force-pushed the percent-encode-query-special-chars branch 2 times, most recently from 786c6e8 to 808aeeb Compare September 26, 2022 16:02
headers cannot contain special characters such as newlines
(as is described in GH issue #104). The solution is to
percent-encode (aka: encodeURIComponent()) the query string.
Encoding is not mandatory, some characters like the space
are HTTP header-safe, these characters can be sent both
as they are or percent encoded (e.g. %20 for space).
@neumark neumark force-pushed the percent-encode-query-special-chars branch from 3be0e3d to 5d2c6f3 Compare September 26, 2022 19:43
@neumark neumark merged commit 09d426c into main Sep 26, 2022
@neumark neumark deleted the percent-encode-query-special-chars branch September 26, 2022 20:48
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.

2 participants