What about rethinking the logic a bit? This is only needed for error responses that echo back to the client a piece of information it already knows.

It seems like each case could be simplified by removing the variable portion of each error string.

Removing the variable from the error string would also be feasible method IMO.
Depends if we'd like to keep the "more information" in the error string.
But i agree with just have "invalid hash" instead of "invalid hash: /inputhash/", etc.

On the other hand it could be handy and save(r) for upcoming implementations to make sure input parameters gets a minimalistic check.

In the current implementation there is a minimalistic very tiny risk of exploiting IsHex (eating cpu over rest with horrible path lengths, path_max is not specified in RFC as far as i know).

Somehow i have a feeling we should truncate and SanitizeString() all input parameters to prevent any possible upcoming damage.

+1 for better input validation.

However I don't like the approach of using sanitize for input strings. It should either accept the input as-is or reject it and throw an error immediately. Don't try to proceed at all using 'sanitized' input.

Using sanitizestring for logged messages makes sense.

I agree with @laanwj -- throw an error if given unsanitary input, don't try to fix it.
(but sanitize if input is logged as part of the error handling, of course)

Agreed. I'll update the PR to detect unsanitary inputs and throw exception in such cases.

Updated.
Now it will detect unsafe inputs. Outputs still go though SanitizeString().

@laanwj: Right. Is was redundant.

I just updated the whole PR.
I did also remove SanitizeString() from error replies because we are now sure that hashes and amounts are correct formatted, etc.
How does it look now?

The redundant CheckRequestString check still seems to be there?

I'd personally have kept the SanitizeStrings on output just in case, as a kind of belt-and-suspenders, but no strong opinion there.

@laanwj There is no more redundancy. CheckRequestString() will ensure input correctness, ParseHashStr() will no longer check for correctness of the hashstr to parse.
I removed SanitizeStrings because it was no longer possible to output a non hex hash or unlimited-non-digi-value.

ParseHashStr() will no longer check for correctness of the hashstr to parse.

To me it seems that was an excellent place to do the check. Where possible, combining checking input and parsing makes sense, as it reduces the scope for error.

I'm also not happy with the solution. Let me try to optimize and shuffle around some things.