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

Incorrect response codes for HTTP proxy #1987

Open
Vort opened this issue Dec 5, 2023 · 5 comments
Open

Incorrect response codes for HTTP proxy #1987

Vort opened this issue Dec 5, 2023 · 5 comments

Comments

@Vort
Copy link
Contributor

Vort commented Dec 5, 2023

When internal i2pd HTTP proxy fails to make connection, it returns response with error code 500.
This is wrong, more suitable code is 503.
Most likely, other errors have wrong codes too.

This is how Host is down case is processed now:

i2pd/libi2pd_client/HTTPProxy.cpp

Line 773 in c215f2e

GenericProxyError(tr("Host is down"), tr("Can't create connection to requested host, it may be down. Please try again later."));

i2pd/libi2pd_client/HTTPProxy.cpp

Lines 165 to 171 in c215f2e

void HTTPReqHandler::GenericProxyError(const std::string& title, const std::string& description) {
std::stringstream ss;
ss << "<h1>" << tr("Proxy error") << ": " << title << "</h1>\r\n";
ss << "<p>" << description << "</p>\r\n";
std::string content = ss.str();
SendProxyError(content);
}

i2pd/libi2pd_client/HTTPProxy.cpp

Lines 198 to 212 in c215f2e

void HTTPReqHandler::SendProxyError(std::string& content)
{
i2p::http::HTTPRes res;
res.code = 500;
res.add_header("Content-Type", "text/html; charset=UTF-8");
res.add_header("Connection", "close");
std::stringstream ss;
ss << "<html>\r\n" << pageHead
<< "<body>" << content << "</body>\r\n"
<< "</html>\r\n";
res.body = ss.str();
m_Response = res.to_string();
boost::asio::async_write(*m_sock, boost::asio::buffer(m_Response), boost::asio::transfer_all(),
std::bind(&HTTPReqHandler::SentHTTPFailed, shared_from_this(), std::placeholders::_1));
}

@zzzi2p
Copy link

zzzi2p commented Dec 18, 2023

We use 500 Domain Not Found if the hostname is not in the addressbook and 504 Gateway TImeout if the host is apparently down (no LS or no response). Here's our error pages for reference: https://github.com/i2p/i2p.i2p/tree/master/apps/i2ptunnel/resources/proxy

@Vort
Copy link
Contributor Author

Vort commented Dec 18, 2023

It means that Java I2P requires fixing as well.

Simply speaking, 500 error should appear only when something unexpected happened.
When condition leading to error is known to developer, some other code is needed.

The HyperText Transfer Protocol (HTTP) 500 Internal Server Error server error response code indicates that the server encountered an unexpected condition that prevented it from fulfilling the request.
This error response is a generic "catch-all" response. Usually, this indicates the server cannot find a better 5xx error code to response.

https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/500

@zzzi2p
Copy link

zzzi2p commented Dec 19, 2023

We have 20+ error pages and we only use 500 for the one case I mentioned - hostname not in address book. You have a suggestion for what would be better for that?

@Vort
Copy link
Contributor Author

Vort commented Dec 19, 2023

we only use 500 for the one case I mentioned - hostname not in address book. You have a suggestion for what would be better for that?

This is what I found:

Most I have seen return 502 Bad Gateway or 504 Gateway Timeout.

https://stackoverflow.com/questions/24395931/what-to-return-to-browser-if-dns-lookup-failed#comment37735176_24395931

unfortunately from time to time we get 504 error DNS lookup failed

https://community.cloudflare.com/t/cant-resolve-nationalbank-kz-name-proxy-504-dns-lookup-failed/54835

@zzzi2p
Copy link

zzzi2p commented Dec 22, 2023

changed to 502

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Vort @zzzi2p