Skip to content
This repository has been archived by the owner on Mar 12, 2020. It is now read-only.

implement http 400 bad request #1119

Closed
wants to merge 2 commits into from
Closed

Conversation

howjmay
Copy link
Member

@howjmay howjmay commented Apr 22, 2019

Add arg status_code to error_serialize_response()
Return status code 400 if no message and no status code was specified.

fixes #1085

@thibault-martinez
Copy link
Member

Sorry for the delay, I had to think about this one. Will review later today !

retcode_t ret = RC_OK;

if (*error == NULL) {
if (message == NULL) {
if (message == NULL && status_code == MHD_HTTP_INTERNAL_SERVER_ERROR) {
Copy link
Member

Choose a reason for hiding this comment

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

Why 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.

I originally thought I should use Bad request as dfault error status code.
I will change && to ||

Copy link
Member

Choose a reason for hiding this comment

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

I don't think it's necessary at all

Copy link
Member Author

@howjmay howjmay May 7, 2019

Choose a reason for hiding this comment

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

what should I do?

Copy link
Member Author

Choose a reason for hiding this comment

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

remove message == NULL

Copy link
Member

Choose a reason for hiding this comment

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

Remove && status_code == MHD_HTTP_INTERNAL_SERVER_ERROR

Copy link
Member Author

Choose a reason for hiding this comment

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

OK

retcode_t ret = RC_OK;

if (*error == NULL) {
if (message == NULL) {
if (message == NULL && status_code == MHD_HTTP_INTERNAL_SERVER_ERROR) {
*error = error_res_new("Internal server error");
Copy link
Member

Choose a reason for hiding this comment

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

Can you add status_code as parameter to error_res_new ?

ciri/api/http/http.c Outdated Show resolved Hide resolved
@thibault-martinez
Copy link
Member

Since you opened this PR, I started working on a gRPC API.
This file will probably change a lot, so please address comments, fix conflicts and I'm not sure when I'll be able to merge it, sorry about that !

@howjmay
Copy link
Member Author

howjmay commented May 7, 2019

Since you opened this PR, I started working on a gRPC API.
This file will probably change a lot, so please address comments, fix conflicts and I'm not sure when I'll be able to merge it, sorry about that !

got it! cheer up

@thibault-martinez
Copy link
Member

Won't be merged due to ciri being discontinued and entangled soon being archived.
Thanks anyway!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

api/http: send HTTP code 400 on errors
2 participants