Skip to content

Commit 0495d1c

Browse files
YassinEldeebYassinEldeeb
committed
fix(server/http): graphql server compliance issues
1 parent 82587bd commit 0495d1c

File tree

3 files changed

+111
-31
lines changed

3 files changed

+111
-31
lines changed

‎.gitignore

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,7 @@ lcov.info
2828
# Built solidity contracts.
2929
/tests/**/bin
3030
/tests/**/truffle_output
31+
32+
# Docker volumes and debug logs
33+
.postgres
34+
logfile

‎server/http/src/server.rs

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
use std::net::{Ipv4Addr, SocketAddrV4};
22

3-
use hyper::service::make_service_fn;
4-
use hyper::Server;
3+
use futures::future::{ok, Future};
4+
use hyper::service::{make_service_fn, service_fn};
5+
use hyper::{Body, Method, Request, Response, Server, StatusCode};
6+
use thiserror::Error;
57

68
use crate::service::GraphQLService;
7-
use graph::prelude::{GraphQLServer as GraphQLServerTrait, *};
8-
use thiserror::Error;
9+
use graph::prelude::{GraphQLServer as GraphQLServerTrait, GraphQlRunner, *};
910

1011
/// Errors that may occur when starting the server.
1112
#[derive(Debug, Error)]
@@ -66,12 +67,27 @@ where
6667
let graphql_runner = self.graphql_runner.clone();
6768
let node_id = self.node_id.clone();
6869
let new_service = make_service_fn(move |_| {
69-
futures03::future::ok::<_, Error>(GraphQLService::new(
70-
logger_for_service.clone(),
71-
graphql_runner.clone(),
72-
ws_port,
73-
node_id.clone(),
74-
))
70+
ok::<_, Error>(service_fn(move |req: Request<Body>| {
71+
match (
72+
req.method(),
73+
req.headers().get(hyper::header::CONTENT_LENGTH),
74+
) {
75+
(&Method::POST, Some(length)) if length == "0" => {
76+
let mut res = Response::new(Body::empty());
77+
*res.status_mut() = StatusCode::BAD_REQUEST;
78+
ok::<_, Error>(res)
79+
}
80+
_ => {
81+
let service = GraphQLService::new(
82+
logger_for_service.clone(),
83+
graphql_runner.clone(),
84+
ws_port,
85+
node_id.clone(),
86+
);
87+
futures03::future::ok::<_, Error>(service)
88+
}
89+
}
90+
}))
7591
});
7692

7793
// Create a task to run the server and handle HTTP requests

‎server/http/src/service.rs

Lines changed: 81 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ use std::task::Context;
44
use std::task::Poll;
55
use std::time::Instant;
66

7+
use graph::prelude::serde_json;
8+
use graph::prelude::serde_json::json;
79
use graph::prelude::*;
810
use graph::semver::VersionReq;
911
use graph::{components::server::query::GraphQLServerError, data::query::QueryTarget};
@@ -62,14 +64,17 @@ where
6264
}
6365

6466
async fn index(self) -> GraphQLServiceResult {
67+
let response_obj = json!({
68+
"message": "Access deployed subgraphs by deployment ID at \
69+
/subgraphs/id/<ID> or by name at /subgraphs/name/<NAME>"
70+
});
71+
let response_str = serde_json::to_string(&response_obj).unwrap();
72+
6573
Ok(Response::builder()
6674
.status(200)
6775
.header(ACCESS_CONTROL_ALLOW_ORIGIN, "*")
68-
.header(CONTENT_TYPE, "text/plain")
69-
.body(Body::from(String::from(
70-
"Access deployed subgraphs by deployment ID at \
71-
/subgraphs/id/<ID> or by name at /subgraphs/name/<NAME>",
72-
)))
76+
.header(CONTENT_TYPE, "application/json")
77+
.body(Body::from(response_str))
7378
.unwrap())
7479
}
7580

@@ -79,7 +84,7 @@ where
7984
Ok(Response::builder()
8085
.status(200)
8186
.header(ACCESS_CONTROL_ALLOW_ORIGIN, "*")
82-
.header(CONTENT_TYPE, "text/html")
87+
.header(CONTENT_TYPE, "text/html; charset=utf-8")
8388
.body(Body::from(contents))
8489
.unwrap())
8590
}
@@ -202,7 +207,7 @@ where
202207
.header(ACCESS_CONTROL_ALLOW_ORIGIN, "*")
203208
.header(ACCESS_CONTROL_ALLOW_HEADERS, "Content-Type, User-Agent")
204209
.header(ACCESS_CONTROL_ALLOW_METHODS, "GET, OPTIONS, POST")
205-
.header(CONTENT_TYPE, "text/html")
210+
.header(CONTENT_TYPE, "text/html; charset=utf-8")
206211
.body(Body::from(""))
207212
.unwrap())
208213
}
@@ -220,7 +225,7 @@ where
220225
.status(StatusCode::FOUND)
221226
.header(ACCESS_CONTROL_ALLOW_ORIGIN, "*")
222227
.header(LOCATION, loc_header_val)
223-
.header(CONTENT_TYPE, "text/plain")
228+
.header(CONTENT_TYPE, "text/plain; charset=utf-8")
224229
.body(Body::from("Redirecting..."))
225230
.unwrap()
226231
})
@@ -229,11 +234,16 @@ where
229234
/// Handles 404s.
230235
fn handle_not_found(&self) -> GraphQLServiceResponse {
231236
async {
237+
let response_obj = json!({
238+
"message": "Not found"
239+
});
240+
let response_str = serde_json::to_string(&response_obj).unwrap();
241+
232242
Ok(Response::builder()
233-
.status(StatusCode::NOT_FOUND)
234-
.header(CONTENT_TYPE, "text/plain")
243+
.status(200)
244+
.header(CONTENT_TYPE, "application/json")
235245
.header(ACCESS_CONTROL_ALLOW_ORIGIN, "*")
236-
.body(Body::from("Not found"))
246+
.body(Body::from(response_str))
237247
.unwrap())
238248
}
239249
.boxed()
@@ -316,30 +326,43 @@ where
316326
// Instead, we generate a Response with an error code and return Ok
317327
Box::pin(async move {
318328
let result = service.handle_call(req).await;
329+
319330
match result {
320331
Ok(response) => Ok(response),
321-
Err(err @ GraphQLServerError::ClientError(_)) => Ok(Response::builder()
322-
.status(400)
323-
.header(CONTENT_TYPE, "text/plain")
324-
.header(ACCESS_CONTROL_ALLOW_ORIGIN, "*")
325-
.body(Body::from(err.to_string()))
326-
.unwrap()),
332+
Err(err @ GraphQLServerError::ClientError(_)) => {
333+
let response_obj = json!({
334+
"error": err.to_string()
335+
});
336+
let response_str = serde_json::to_string(&response_obj).unwrap();
337+
338+
Ok(Response::builder()
339+
.status(200)
340+
.header(CONTENT_TYPE, "application/json")
341+
.header(ACCESS_CONTROL_ALLOW_ORIGIN, "*")
342+
.body(Body::from(response_str))
343+
.unwrap())
344+
}
327345
Err(err @ GraphQLServerError::QueryError(_)) => {
328346
error!(logger, "GraphQLService call failed: {}", err);
329347

348+
let response_obj = json!({
349+
"QueryError": err.to_string()
350+
});
351+
let response_str = serde_json::to_string(&response_obj).unwrap();
352+
330353
Ok(Response::builder()
331-
.status(400)
332-
.header(CONTENT_TYPE, "text/plain")
354+
.status(200)
355+
.header(CONTENT_TYPE, "application/json")
333356
.header(ACCESS_CONTROL_ALLOW_ORIGIN, "*")
334-
.body(Body::from(format!("Query error: {}", err)))
357+
.body(Body::from(response_str))
335358
.unwrap())
336359
}
337360
Err(err @ GraphQLServerError::InternalError(_)) => {
338361
error!(logger, "GraphQLService call failed: {}", err);
339362

340363
Ok(Response::builder()
341364
.status(500)
342-
.header(CONTENT_TYPE, "text/plain")
365+
.header(CONTENT_TYPE, "text/plain; charset=utf-8")
343366
.header(ACCESS_CONTROL_ALLOW_ORIGIN, "*")
344367
.body(Body::from(format!("Internal server error: {}", err)))
345368
.unwrap())
@@ -352,7 +375,9 @@ where
352375
#[cfg(test)]
353376
mod tests {
354377
use graph::data::value::Object;
378+
use http::header::CONTENT_TYPE;
355379
use http::status::StatusCode;
380+
use hyper::body::HttpBody;
356381
use hyper::service::Service;
357382
use hyper::{Body, Method, Request};
358383

@@ -423,6 +448,39 @@ mod tests {
423448
}
424449
}
425450

451+
#[tokio::test]
452+
async fn querying_not_found_routes_responds_correctly() {
453+
let logger = Logger::root(slog::Discard, o!());
454+
let graphql_runner = Arc::new(TestGraphQlRunner);
455+
456+
let node_id = NodeId::new("test").unwrap();
457+
let mut service = GraphQLService::new(logger, graphql_runner, 8001, node_id);
458+
459+
let request = Request::builder()
460+
.method(Method::GET)
461+
.header(CONTENT_TYPE, "text/plain; charset=utf-8")
462+
.uri("http://localhost:8000/not_found_route".to_string())
463+
.body(Body::from("{}"))
464+
.unwrap();
465+
466+
let response =
467+
futures03::executor::block_on(service.call(request)).expect("Should return a response");
468+
469+
let content_type_header = response.status();
470+
assert_eq!(content_type_header, StatusCode::OK);
471+
472+
let content_type_header = response.headers().get(CONTENT_TYPE).unwrap();
473+
assert_eq!(content_type_header, "application/json");
474+
475+
let body_bytes = hyper::body::to_bytes(response.into_body()).await.unwrap();
476+
let json: serde_json::Result<serde_json::Value> =
477+
serde_json::from_str(String::from_utf8(body_bytes.to_vec()).unwrap().as_str());
478+
479+
assert!(json.is_ok(), "Response body is not valid JSON");
480+
481+
assert_eq!(json.unwrap(), serde_json::json!({"message": "Not found"}));
482+
}
483+
426484
#[test]
427485
fn posting_invalid_query_yields_error_response() {
428486
let logger = Logger::root(slog::Discard, o!());
@@ -434,6 +492,7 @@ mod tests {
434492

435493
let request = Request::builder()
436494
.method(Method::POST)
495+
.header(CONTENT_TYPE, "text/plain; charset=utf-8")
437496
.uri(format!(
438497
"http://localhost:8000/subgraphs/id/{}",
439498
subgraph_id
@@ -464,6 +523,7 @@ mod tests {
464523

465524
let request = Request::builder()
466525
.method(Method::POST)
526+
.header(CONTENT_TYPE, "text/plain; charset=utf-8")
467527
.uri(format!(
468528
"http://localhost:8000/subgraphs/id/{}",
469529
subgraph_id

0 commit comments

Comments
 (0)