Hi, we have a problem with the Tapestry URLEncoder and how invalid characters are handled in the ActivationRequestParameterWorker.
If a component request URL contains unsafe characters, an IllegalArgumentException is thrown: https://github.com/apache/tapestry-5/blob/b347e650ebf1204b87220166c7d126129b5ee460/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/URLEncoderImpl.java#L144 Our logging setup sends a mail for every unhandled exception, and we receive quite a lot of these exceptions due to dumb bots trying random URLS. The strictness of the URLEncoder was discussed before, and the ticket is still open: https://issues.apache.org/jira/browse/TAP5-1803 The reasoning behind using a custom URLEncode and not the Java URLEncoder is still sound IMO and shouldn't be changed. We internally discussed how we could mitigate the problem, and the following ideas came to mind: Idea 1: @ActivationRequestParameter should have "emptyOnError" with a default value of false. If the decode fails, this option would set the decoded String to empty String. The value encoder would create a valid value for the component, and no 500 error would occur. There shouldn't be any backward compatibility issues due to the default value "false". But the new option would allow us to handle invalid values by actually receiving a value in any case. The idea sounded good until I tested it, and an empty string isn't leading to valid values from the ValueEncoder. For example, in the case of numeric values, it would just trade the IllegalArgumentException with NumberFormatException. Idea 2: Stop the request and return an HTTP 400. Why does an unencodable URL have to lead to an HTTP 500 status? >From an HTTP workflow point of view, the server receives a request it can't handle, so an HTTP 400 would be a better option because the request is faulty. Catching any IllegalArgumentException in https://github.com/apache/tapestry-5/blob/b347e650ebf1204b87220166c7d126129b5ee460/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ActivationRequestParameterWorker.java#L136 and stop the request and return status 400. The first problem we thought about is how to interrupt the request at that particular point. The crash, in our case, is on page activation in ComponentEventRequestHandlerImpl. There should be mechanism added to allow the page activation to tell the ComponentEventRequestHandlerImpl that the activation failed due to "bad request", and not a server error. What do you think? Do you have similar issues with bots and invalid requests? Are we overthinking it, and there's a more straightforward way to deal with the problem? Cheers Ben
