forked from jl777/SuperNET
-
Notifications
You must be signed in to change notification settings - Fork 98
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
fix(cors): allow OPTIONS request to KDF server #2191
Merged
Merged
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
d2349df
add OPTIONS request method
shamardy 49e18bb
return non-allowed characters in the response error message
shamardy 7b6eb70
Include CORS headers in error responses for rpc_service function
shamardy 3dc9715
fix: remove unneeded clone
shamardy 6f144e9
fix: revert to using 3000 port as default port for cors
shamardy 27e687c
fix: remove unneeded clone
shamardy 570bd79
Review fix: Scope some variables for early release in `rpc_service` fn
shamardy 2d3e268
Review fix: Removed one clone while scoping more vars for `body_escaped`
shamardy 9b94bf0
fix ci docker tests, use v0.8.1 komodo release
shamardy File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -235,7 +235,7 @@ async fn process_single_request(ctx: MmArc, req: Json, client: SocketAddr) -> Re | |
} | ||
|
||
#[cfg(not(target_arch = "wasm32"))] | ||
async fn rpc_service(req: Request<Body>, ctx_h: u32, client: SocketAddr) -> Response<Body> { | ||
async fn rpc_service(req: Request<Body>, ctx_h: u32, client: SocketAddr, local_port: u16) -> Response<Body> { | ||
/// Unwraps a result or propagates its error 500 response with the specified headers (if they are present). | ||
macro_rules! try_sf { | ||
($value: expr $(, $header_key:expr => $header_val:expr)*) => { | ||
|
@@ -269,11 +269,30 @@ async fn rpc_service(req: Request<Body>, ctx_h: u32, client: SocketAddr) -> Resp | |
// https://github.com/artemii235/SuperNET/issues/219 | ||
let rpc_cors = match ctx.conf["rpccors"].as_str() { | ||
Some(s) => try_sf!(HeaderValue::from_str(s)), | ||
None => HeaderValue::from_static("http://localhost:3000"), | ||
None => { | ||
let rpc_cors = if ctx.is_https() { | ||
format!("https://localhost:{}", local_port) | ||
} else { | ||
format!("http://localhost:{}", local_port) | ||
}; | ||
try_sf!(HeaderValue::from_str(&rpc_cors)) | ||
}, | ||
}; | ||
|
||
// Convert the native Hyper stream into a portable stream of `Bytes`. | ||
let (req, req_body) = req.into_parts(); | ||
|
||
if req.method == Method::OPTIONS { | ||
return Response::builder() | ||
.status(StatusCode::OK) | ||
.header(ACCESS_CONTROL_ALLOW_ORIGIN, rpc_cors.clone()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed here 3dc9715 |
||
.header("Access-Control-Allow-Methods", "POST, OPTIONS") | ||
shamardy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
.header("Access-Control-Allow-Headers", "Content-Type") | ||
.header("Access-Control-Max-Age", "3600") | ||
.body(Body::empty()) | ||
.unwrap(); | ||
} | ||
|
||
let req_bytes = try_sf!(hyper::body::to_bytes(req_body).await, ACCESS_CONTROL_ALLOW_ORIGIN => rpc_cors); | ||
let req_str = String::from_utf8_lossy(req_bytes.as_ref()); | ||
let is_invalid_input = req_str.chars().any(|c| c == '<' || c == '>' || c == '&'); | ||
|
@@ -361,14 +380,19 @@ pub extern "C" fn spawn_rpc(ctx_h: u32) { | |
|
||
let is_event_stream_enabled = ctx.event_stream_configuration.is_some(); | ||
|
||
let rpc_ip_port = ctx | ||
.rpc_ip_port() | ||
.unwrap_or_else(|err| panic!("Invalid RPC port: {}", err)); | ||
let port = rpc_ip_port.port(); | ||
|
||
let make_svc_fut = move |remote_addr: SocketAddr| async move { | ||
Ok::<_, Infallible>(service_fn(move |req: Request<Body>| async move { | ||
if is_event_stream_enabled && req.uri().path() == SSE_ENDPOINT { | ||
let res = handle_sse(req, ctx_h).await?; | ||
return Ok::<_, Infallible>(res); | ||
} | ||
|
||
let res = rpc_service(req, ctx_h, remote_addr).await; | ||
let res = rpc_service(req, ctx_h, remote_addr, port).await; | ||
Ok::<_, Infallible>(res) | ||
})) | ||
}; | ||
|
@@ -432,9 +456,6 @@ pub extern "C" fn spawn_rpc(ctx_h: u32) { | |
}; | ||
} | ||
|
||
let rpc_ip_port = ctx | ||
.rpc_ip_port() | ||
.unwrap_or_else(|err| panic!("Invalid RPC port: {}", err)); | ||
// By entering the context, we tie `tokio::spawn` to this executor. | ||
let _runtime_guard = CORE.0.enter(); | ||
|
||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
maybe we want to abandon
rpccors
now?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.
I realize now that this port should be the frontend port used in development. I can use the client port here instead, but this opens it up for any port the request comes from, e.g., a script running locally on any port in the browser can access the RPC service. I will revert to port 3000 as it's commonly used for frontend development.
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.
Fixed here 6f144e9