Improved handling for HTTP X-Forwarded-For and Forwarded #4009
Conversation
|
good |
|
|
||
| return ret; | ||
| } | ||
|
|
There was a problem hiding this comment.
Would boost::regex be better? It's certainly a bit less code.
There was a problem hiding this comment.
This was a good question to ask. At this prompt I explored using boost::regex to do the work. I don't have a lot of experience with boost::regex, so maybe I'm missing something. But from what I can see boost::regex does not support boost::string_view very well if at all.
The problem is that ripple::forwardedFor() is designed to return a boost::string_view extracted from the passed in http_request_type const&. I believe this design is intended to avoid the memory allocations that are involved with std::string creation. After poking at boost::regex for a few hours I've been unable to locate the operations I would need to perform in order to get boost::regex to operate on and produce the needed boost::string_view. A few web searches helped to reinforce this suspicion.
Maybe you have more experience with boost::regex and can steer me in the right direction? Failing that, I'm afraid the current character manipulations are the best I can do.
I did not look into using std::regex since, disappointingly, std::regex is not portable across platforms.
There was a problem hiding this comment.
I see. I didn't consider boost::string_view.
|
Thanks for the reviews, folks. The Travis CI failures are our usual build failures on macOS. I'm going to mark this pull request as passed. |
scottschurr
added
the
Passed
Improve handling of HTTP
X-Forwarded-ForandForwardedfieldsIt was discovered that rippled was not handling IPv6 and IPv6 (dual) formatted IP addresses in
X-Forwarded-ForandForwardedfields. This commit improves the situation and adds tests to verify.Context of Change
The problem was discovered when services were forwarding requests to reporting mode servers. IPV6 formatted addresses were not being handled correctly. The upshot of the change should simply be that addresses which failed before should now be handled correctly.
Type of Change
Test Plan
The unit tests simply confirm that the
X-Forwarded-ForandForwardedfields are isolated correctly and forms a valid IP address.It may be smart to add integration tests that use
X-Forwarded-ForandForwardedfields so the changes can be exercised in an actual running environment.