You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "mkmik (via GitHub)" <gi...@apache.org> on 2023/05/23 19:35:09 UTC
[GitHub] [arrow-rs] mkmik opened a new pull request, #4271: Strip leading whitespace from flight_sql_client custom header values
mkmik opened a new pull request, #4271:
URL: https://github.com/apache/arrow-rs/pull/4271
# Which issue does this PR close?
Closes #4270
# What changes are included in this PR?
This PR fixes the code that parses the `key: value` pairs when parsing the `--headers` flag
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] tustvold merged pull request #4271: Strip leading whitespace from flight_sql_client custom header values
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold merged PR #4271:
URL: https://github.com/apache/arrow-rs/pull/4271
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] mkmik commented on a diff in pull request #4271: Strip leading whitespace from flight_sql_client custom header values
Posted by "mkmik (via GitHub)" <gi...@apache.org>.
mkmik commented on code in PR #4271:
URL: https://github.com/apache/arrow-rs/pull/4271#discussion_r1204463229
##########
arrow-flight/src/bin/flight_sql_client.rs:
##########
@@ -49,7 +49,7 @@ where
match parts.as_slice() {
[key, value] => {
let key = K::from_str(key).map_err(|e| e.to_string())?;
- let value = V::from_str(value).map_err(|e| e.to_string())?;
+ let value = V::from_str(value.trim_start()).map_err(|e| e.to_string())?;
Review Comment:
Added commit that switches to `.trim()`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] alamb commented on a diff in pull request #4271: Strip leading whitespace from flight_sql_client custom header values
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #4271:
URL: https://github.com/apache/arrow-rs/pull/4271#discussion_r1204117418
##########
arrow-flight/src/bin/flight_sql_client.rs:
##########
@@ -49,7 +49,7 @@ where
match parts.as_slice() {
[key, value] => {
let key = K::from_str(key).map_err(|e| e.to_string())?;
- let value = V::from_str(value).map_err(|e| e.to_string())?;
+ let value = V::from_str(value.trim_start()).map_err(|e| e.to_string())?;
Review Comment:
Should we also be trimming trailing whitespace?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] mkmik commented on a diff in pull request #4271: Strip leading whitespace from flight_sql_client custom header values
Posted by "mkmik (via GitHub)" <gi...@apache.org>.
mkmik commented on code in PR #4271:
URL: https://github.com/apache/arrow-rs/pull/4271#discussion_r1204462482
##########
arrow-flight/src/bin/flight_sql_client.rs:
##########
@@ -49,7 +49,7 @@ where
match parts.as_slice() {
[key, value] => {
let key = K::from_str(key).map_err(|e| e.to_string())?;
- let value = V::from_str(value).map_err(|e| e.to_string())?;
+ let value = V::from_str(value.trim_start()).map_err(|e| e.to_string())?;
Review Comment:
Yeah I thought only leading whitespace was trimmed in HTTP/1.1 (upon which the CLI tools like curl are modelled on), but it turns out that trailing whitespace is optioinal too:
https://www.rfc-editor.org/rfc/rfc7230#appendix-B
```
OWS = *( SP / HTAB )
header-field = field-name ":" OWS field-value OWS
field-value = *( field-content / obs-fold )
field-content = field-vchar [ 1*( SP / HTAB ) field-vchar ]
field-vchar = VCHAR / obs-text
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org