-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Description
Describe the bug
HTTPStreamFactory::open
does not behave as documented. There, it states
Redirect responses are handled and the redirect location is automatically resolved, as long as the redirect location is still accessible via the HTTP protocol.
If a redirection to a non http://../. URI is received, aUnsupportedRedirectException
exception is thrown.
The offending URI can then be obtained via themessage()
method ofUnsupportedRedirectException
.
In fact, the problems are:
- Redirects are not handled automatically, a
URIRedirection
"exception" is thrown. - This class is not derived from
Poco::Exception
and thus only for internal use. It is handled byURIStreamOpener::openURI
. - No
UnsupportedRedirectException
is ever thrown.
To Reproduce
Set up an HTTP server with a redirection to an "unknown" protocol (note that this would be against best practices, would likely cause problems with most clients, and such severs should normally never be set up and especially not put in productive use). Use classes from Poco's Net component to connect to this server.
Please note: due to lack of access to such a server, I did not actually run any test. I only consulted documentation and code.
Expected behavior
Either: See a UnsupportedRedirectException
exception being thrown for the documented case.
Or: Have an improved documentation which explains the actual behavior (including any documentation for the UnknownURISchemeException
exception).
Also, you might consider to do throw an UnsupportedRedirectException
for redirects which might be considered unsupported despite the target protocol being known, such as a https-to-http (sic!) redirect which can be considered a security breach.
Additional context
According to the changelog, this behavior should have been introduced with v1.2.0
Release 1.2.0 (2006-08-29)
[...]
- added Net::UnsupportedRedirectException
- HTTPStreamFactory throws an UnsupportedRedirectException if it encounters a redirect to https
This (super old) change for v1.3.2 with changelog entry
Release 1.3.2 (2008-02-04)
[...]
- URIStreamOpener improvement: redirect logic is now in URIStreamOpener.
this enables support for redirects from http to https.
actually removes the actual throw of this exception from HTTPStreamFactory::open
- but there is no equivalent throw afterwards in the URIStreamOpener
class. So in fact, this enables any redirect between "registered" protocols. So at least from http to https and vice versa. For a non-registered protocols, a UnknownURISchemeException
exception is thrown by URIStreamOpener::open
.
There might be other problems in HTTPStreamFactory::open
as well: currently, there is a *dead code path which throws a HTTPException("Too many redirects", ...)
exception. (Besided being a dead path, this is awkward given that URIStreamOpener::openURI
throws TooManyURIRedirectsException
if applicable.)