Accept all valid currency codes in API #4566
Conversation
A few methods, including `book_offers`, take currency codes as parameters. The XRPL doesn't care if the letters in those codes are lowercase or uppercase, as long as they come from an alphabet defined internally. rippled doesn't care either, when they are submitted in a hex representation. When they are submitted in an ASCII string representation, rippled, but not XRPL, is more restrictive, preventing clients from interacting with some currencies already in the XRPL. This change gets rippled out of the way and lets clients submit currency codes in ASCII using the full alphabet. Fixes XRPLF#4112
| @@ -121,7 +121,8 @@ class RPCParser | |||
| static Json::Value | |||
| jvParseCurrencyIssuer(std::string const& strCurrencyIssuer) | |||
| { | |||
| static boost::regex reCurIss("\\`([[:alpha:]]{3})(?:/(.+))?\\'"); | |||
| static boost::regex reCurIss( | |||
| "\\`([][:alnum:]<>(){}[|?!@#$%^&*]{3})(?:/(.+))?\\'"); | |||
There was a problem hiding this comment.
How is this regex working correctly? Is the tilde character (third character from the beginning) replaceable with the single-quote (last but one character) inside the regex?
can we include a comment referring to full alphabet or reuse the detail::isoCharSet variable? [I was thinking: "\`((detail::isoCharSet){3})(?:/(.+))?\'", but I'm not sure]
also, I've used regex libraries in other languages, but this one baffles me. what is the purpose of the suffix of this regular expression? [?:/(.+))?]
There was a problem hiding this comment.
This is a boost::regex. It uses Perl syntax, which the library documents online. Look at the section "Buffer boundaries". (The character ` is called a backtick; tilde is ~.) \` matches the start of a buffer and \' matches the end of a buffer, so no, they cannot be replaced.
You could try to construct a regex string where detail::isoCharSet sits in a character class, but you would have to change the order within detail::isoCharSet because ] cannot appear in the middle of a character class; it can only appear as the first character in the class. I don't think that factoring is worth it, though.
(?:...) is a non-capturing group. It groups subatoms into a larger atom without creating a capture group. So (?:/(.+))? optionally matches a forward slash and a non-empty sequence of characters, capturing the characters after the slash into a group.
There was a problem hiding this comment.
ok, this is helpful. thank you
There was a problem hiding this comment.
I still feel adding a comment would be helpful for future readers of the code, but it's upto you.
On that note, how do you include suggested patches in github comments? I remember you did something like that for me a while ago.
There was a problem hiding this comment.
| "\\`([][:alnum:]<>(){}[|?!@#$%^&*]{3})(?:/(.+))?\\'"); | |
| // The below regex matches the pattern as specified in UIntTypes.cpp:detail::isoCharSet. It is utilizes boost::regex and adheres to the Perl syntax for regular expressions. | |
| "\\`([][:alnum:]<>(){}[|?!@#$%^&*]{3})(?:/(.+))?\\'"); |
There was a problem hiding this comment.
@thejohnfreeman to accept, modify, or decline suggested comment
There was a problem hiding this comment.
I've pushed an alternative comment.
|
lgtm |
thejohnfreeman
added
the
Passed
|
@thejohnfreeman at your convenience, please bring this branch up-to-date with |
A few methods, including `book_offers`, take currency codes as parameters. The XRPL doesn't care if the letters in those codes are lowercase or uppercase, as long as they come from an alphabet defined internally. rippled doesn't care either, when they are submitted in a hex representation. When they are submitted in an ASCII string representation, rippled, but not XRPL, is more restrictive, preventing clients from interacting with some currencies already in the XRPL. This change gets rippled out of the way and lets clients submit currency codes in ASCII using the full alphabet. Fixes XRPLF#4112
A few methods, including
book_offers, take currency codes as parameters. The XRPL doesn't care if the letters in those codes are lowercase or uppercase, as long as they come from an alphabet defined internally. rippled doesn't care either, when they are submitted in a hex representation. When they are submitted in an ASCII string representation, rippled, but not XRPL, is more restrictive, preventing clients from interacting with some currencies already in the XRPL.This change gets rippled out of the way and lets clients submit currency codes in ASCII using the full alphabet.
Fixes #4112