Opened 6 years ago

Closed 6 years ago

#891 closed defect (fixed)

Special symbols in URL parsing

Reported by: james Owned by: str4d
Priority: minor Milestone: 0.9.6
Component: apps/i2ptunnel Version: 0.9.4
Keywords: Cc:
Parent Tickets: Sensitive: no

Description

There is a problem with parsing some special symbols in URL, like |, %, ^; and so on.
Router gives an error "Warning: Non-HTTP Protocol".
Sample problem url: http://forum.i2p/index.php|

Subtickets

Change History (3)

comment:1 Changed 6 years ago by str4d

Component: unspecifiedapps/i2ptunnel

The culprit is Java's URI parser, which (correctly) fails due to the illegal character:

net.i2p.i2ptunnel.I2PTunnelHTTPClient: Bad request [http://forum.i2p/index.php|]

java.net.URISyntaxException: Illegal character in path at index 26: http://forum.i2p/index.php|
at java.net.URI$Parser.fail(URI.java:2809)
at java.net.URI$Parser.checkChars(URI.java:2982)
at java.net.URI$Parser.parseHierarchical(URI.java:3066)
at java.net.URI$Parser.parse(URI.java:3014)
at java.net.URI.<init>(URI.java:578)
at net.i2p.i2ptunnel.I2PTunnelHTTPClient.clientConnectionRun(I2PTunnelHTTPClient.java:412)
at net.i2p.i2ptunnel.I2PTunnelClientBase$BlockingRunner.run(I2PTunnelClientBase.java:687)
at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:908)
at java.lang.Thread.run(Thread.java:662)

This causes I2PTunnelHTTPClient to break out of processing the header, which results in the method, destination and protocol all being null and therefore getting caught by the "Non-HTTP Protocol" error page.

A better solution would be to set the method (as we already know it before parsing the URI) and then to check what caused the URI to fail parsing. We could then possibly attempt to recover, by either ignoring the bad character or truncating anything from the bad character onwards (I don't know if there are any other possible situations where URISyntaxException could be thrown). Or alternatively, throw a specific error page to let the user know that the URI is bad rather than misleading them with the "Non-HTTP Protocol" error page.

comment:2 Changed 6 years ago by str4d

Milestone: 0.9.6
Owner: set to str4d
Status: newaccepted

I'm going to implement the specific error page for now (recovery can always be added in later, with the error page as a fallback).

comment:3 Changed 6 years ago by str4d

Resolution: fixed
Status: acceptedclosed

New error page added in [aed61ed8fee59d664d6f3b7854b4e5421fcc3906] which is correctly triggered for the provided sample URI. I think recovery is outside our scope, so closing the ticket.

Note: See TracTickets for help on using tickets.