Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Adding a Server header to API responses #125

Merged
merged 1 commit into from Dec 21, 2016

Conversation

Projects
None yet
3 participants
Contributor

jstnlef commented Dec 12, 2016

Adding a server header as well as the current CORS headers to responses. This mimics Synapses behavior of adding its server name and version to API responses.

Member

farodin91 commented Dec 13, 2016

Could you add some tests?

Contributor

jstnlef commented Dec 14, 2016

Just added some tests. I didn't see a good example of tests directed specifically toward the middleware so this is what I came up with. Let me know what you think.

Looks great your tests.
Thanks!!!

src/middleware/response_headers.rs
+ remote_addr: "localhost:3000".to_socket_addrs().unwrap().next().unwrap(),
+ local_addr: "localhost:3000".to_socket_addrs().unwrap().next().unwrap(),
+ headers: Headers::new(),
+ body: unsafe { ::std::mem::uninitialized() }, // FIXME(reem): Ugh
@jimmycuadra

jimmycuadra Dec 20, 2016

Owner

I don't want to introduce any unsafe blocks into Ruma, even if Iron does it. Will it work to just use the iron-test crate to test this functionality?

@jstnlef

jstnlef Dec 21, 2016

Contributor

Erm well I could just use iron-test to make a request to an r0 prefixed endpoint and verify that the response has the included headers. It's not quite correct or vigorous but should get the job done. Let me know if I should do that and I will.

@jimmycuadra

jimmycuadra Dec 21, 2016

Owner

That approach sounds fine to me.

Contributor

jstnlef commented Dec 21, 2016

Modified the tests to remove the unsafe bits.

@jimmycuadra jimmycuadra merged commit 5dbf75f into ruma:master Dec 21, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Owner

jimmycuadra commented Dec 21, 2016

Thanks much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment