Skip to content

Commit 4483680

Browse files
YassinEldeebYassinEldeeb
committed
fix(server/http): graphql server compliance issues
1 parent 05e838a commit 4483680

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, Word};
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

@@ -419,6 +444,39 @@ mod tests {
419444
}
420445
}
421446

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

431489
let request = Request::builder()
432490
.method(Method::POST)
491+
.header(CONTENT_TYPE, "text/plain; charset=utf-8")
433492
.uri(format!(
434493
"http://localhost:8000/subgraphs/id/{}",
435494
subgraph_id
@@ -460,6 +519,7 @@ mod tests {
460519

461520
let request = Request::builder()
462521
.method(Method::POST)
522+
.header(CONTENT_TYPE, "text/plain; charset=utf-8")
463523
.uri(format!(
464524
"http://localhost:8000/subgraphs/id/{}",
465525
subgraph_id

0 commit comments

Comments
 (0)