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.

Sign up for GitHub

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

Jump to bottom

fix(native-rpc): remove escaped response body #2219

Merged
merged 2 commits into from
Sep 23, 2024
Merged

fix(native-rpc): remove escaped response body #2219

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f928060
remove escape answeer
borngraced Sep 13, 2024
a391098
trust us and remove regex deps
borngraced Sep 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 11 additions & 47 deletions mm2src/mm2_main/src/rpc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,11 @@ use futures::future::{join_all, FutureExt};
use http::header::{HeaderValue, ACCESS_CONTROL_ALLOW_ORIGIN, CONTENT_TYPE};
use http::request::Parts;
use http::{Method, Request, Response, StatusCode};
use lazy_static::lazy_static;
use mm2_core::mm_ctx::MmArc;
use mm2_err_handle::prelude::*;
use mm2_rpc::mm_protocol::{MmRpcBuilder, MmRpcResponse, MmRpcVersion};
use regex::Regex;
Copy link
Collaborator

Choose a reason for hiding this comment

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

we dropped the only usage of regex here,
we can also drop it in:

komodo-defi-framework/mm2src/mm2_main/Cargo.toml

Lines 80 to 82 in 079ea5e

# TODO: Reduce the size of regex by disabling the features we don't use.
# cf. https://github.com/rust-lang/regex/issues/583
regex = "1"

use serde::Serialize;
use serde_json::{self as json, Value as Json};
use std::borrow::Cow;
use std::net::SocketAddr;

cfg_native! {
Expand Down Expand Up @@ -178,35 +175,6 @@ fn response_from_dispatcher_error(
response.serialize_http_response()
}

pub fn escape_answer<'a, S: Into<Cow<'a, str>>>(input: S) -> Cow<'a, str> {
lazy_static! {
static ref REGEX: Regex = Regex::new("[<>&]").unwrap();
}

let input = input.into();
let mut last_match = 0;

if REGEX.is_match(&input) {
let matches = REGEX.find_iter(&input);
let mut output = String::with_capacity(input.len());
for mat in matches {
let (begin, end) = (mat.start(), mat.end());
output.push_str(&input[last_match..begin]);
match &input[begin..end] {
"<" => output.push_str("&lt;"),
">" => output.push_str("&gt;"),
"&" => output.push_str("&amp;"),
_ => unreachable!(),
}
last_match = end;
}
output.push_str(&input[last_match..]);
Cow::Owned(output)
} else {
input
}
}
Comment on lines -181 to -208
Copy link
Member

Choose a reason for hiding this comment

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

Any idea what was the motivation for this?

Copy link
Member Author

Choose a reason for hiding this comment

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

context: https://matrix.to/#/!yeLsKrHmWqEMEAILHs:matrix.org/$mAxamnv9Ga8rxAg24mXAUwRu9bKQArw2tB_aNaA177c?via=matrix.org


async fn process_single_request(ctx: MmArc, req: Json, client: SocketAddr) -> Result<Response<Vec<u8>>, String> {
let local_only = ctx.conf["rpc_local_only"].as_bool().unwrap_or(true);
if req["mmrpc"].is_null() {
Expand Down Expand Up @@ -314,23 +282,19 @@ async fn rpc_service(req: Request<Body>, ctx_h: u32, client: SocketAddr) -> Resp

let res = try_sf!(process_rpc_request(ctx, req, req_json, client).await, ACCESS_CONTROL_ALLOW_ORIGIN => rpc_cors);
let (mut parts, body) = res.into_parts();
let body_escaped = {
let body_utf8 = match std::str::from_utf8(&body) {
Ok(body_utf8) => body_utf8,
Err(_) => {
return Response::builder()
.status(500)
.header(ACCESS_CONTROL_ALLOW_ORIGIN, rpc_cors)
.header(CONTENT_TYPE, APPLICATION_JSON)
.body(Body::from(err_to_rpc_json_string("Non UTF-8 output")))
.unwrap();
},
};
let escaped = escape_answer(body_utf8);
escaped.as_bytes().to_vec()
let body_bytes = match std::str::from_utf8(&body) {
Ok(body_utf8) => body_utf8.as_bytes().to_vec(),
Err(_) => {
return Response::builder()
.status(500)
.header(ACCESS_CONTROL_ALLOW_ORIGIN, rpc_cors)
.header(CONTENT_TYPE, APPLICATION_JSON)
.body(Body::from(err_to_rpc_json_string("Non UTF-8 output")))
.unwrap();
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we really need to check if the bytes could be utf-8ed at this point?
We should trust us.

};
parts.headers.insert(ACCESS_CONTROL_ALLOW_ORIGIN, rpc_cors);
Response::from_parts(parts, Body::from(body_escaped))
Response::from_parts(parts, Body::from(body_bytes))
}

// TODO: This should exclude TCP internals, as including them results in having to
Expand Down
Loading