ANCHOR-403 Obfuscate Exception in Sep1Helper instead of NetUtil #1052
Conversation
|
Reference Server Preview is available here: |
| if (!response.isSuccessful()) { | ||
| throw new IOException(String.format("Unable to fetch data from %s", url)); |
There was a problem hiding this comment.
Nit: It might be difficult to extract the code from a String. It might be better to introduce a custom Exception class that represents unsuccessful responses and include the code and body as a field. I believe we are only using NetUtil in a couple of places in the code though so we don't really need to block on this IMO.
There was a problem hiding this comment.
thanks. i will file a ticket for the custom exception class due to time constraints and as well jakes comment about return value. can accomodate and improve this together.
There was a problem hiding this comment.
recapturing his comment here
JakeUrban2 hours ago
Are we sure don't want to return 4xx or 5xx response bodies?
If this function only has one reference at the moment, I would suggest changing the return value to include both the response body and status code, that way callers can decide for themselves if they're interested in non-success response bodies.
| @Test | ||
| fun `getStellarToml throws exception when redirect is encountered`() { | ||
| fun `getStellarToml fetches invalid data during malicious re-direct`() { |
There was a problem hiding this comment.
Nit: maybe call this getStellarToml fetches data during redirect?
|
Reference Server Preview is available here: |
|
Something went wrong with PR preview build please check |
|
Reference Server Preview is available here: |
* ANCHOR-403 Obfuscate Exception in Sep1Helper instead of NetUtil * bump minor version * bump version for docs * addressing pr comments
This PR moves the exception obfuscation for the stellar toml file handling to the Toml module and out of NetUtil. The reason is that NetUtil fetch is a lower level function that is shared. Obfuscating the exception message may not be applicable for other usages. THis PR also logs the actual exception message so that it is available for troubleshooting. Added some additional test coverage for Sep1Helper as well.
PR Structure
[x ] This PR has reasonably narrow scope (if not, break it down into smaller PRs).
This PR avoids mixing refactoring changes with feature changes (split into two PRs
otherwise).
[ x] This PR's title starts with name of package that is most changed in the PR, ex.
paymentservice.stellar, or all or doc if the changes are broad or impact many
packages.