-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: Use GCP header for backup localization info #63
Conversation
* Switch outbound headers to middleware * Pull cargo package version from Cargo.toml for dev deploy Review note: This is more about setting up pulling the GCP info from the header. Since GCP does not localize, a proper fix will need to be in more than just "EN-US", but how we address that has not yet been finalized. Issue #44 Closes #59
channelserver/src/meta.rs
Outdated
if let Ok(loc_str) = ghead.to_str() { | ||
let mut bits = loc_str.split(',').collect::<Vec<&str>>(); | ||
let mut bi = bits.iter(); | ||
sender.region = bi.next().map(|s| String::from(*s)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I'd prefer s.to_owned() just to be consistent w/ every where else
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh, mostly because (*s).to_owned()
didn't look a lot better.
channelserver/src/main.rs
Outdated
/// Set the OpSec headers on all responses. | ||
fn response(&self, req: &HttpRequest<S>, mut resp: HttpResponse) -> Result<Response, Error> { | ||
resp.headers_mut().insert( | ||
header::HeaderName::try_from("Content-Security-Policy").unwrap(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about actix_web::middleware::DefaultHeaders (it basically does this)?
if this middleware is necessary:
unfortunately this does require the HeaderValue::from_static I think but you don't need the extra bits for the keys:
header::HeaderName::try_from("Content-Security-Policy").unwrap(), | |
"Content-Security-Policy", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks! looks like i can pass in str for both values.
Review note:
This is more about setting up pulling the GCP info from the header.
Since GCP does not localize, a proper fix will need to be in more than
just "EN-US", but how we address that has not yet been finalized.
Issue #44
Closes #59