-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Added support for XSSI Vulnerability Protected JSON #529
Conversation
Angular js supports this(See this) as well as some json parsing frameworks. |
1 similar comment
@yashsriv Thanks. As far as I understand it though, this vulnerability isn't relevant to our case: HTTPie doesn't have a JavaScript runtime so "overriding native JavaScript object constructors" doesn't apply. Also:
We treat JSON purely as data, and it doesn't get interpreted beyond parsing. |
@jkbrzt I think @yashsriv may have done a poor job explaining their usecase. As I understand their change, they're trying to communicate with a service that returns XSSI protected JSON. Now, with today's HTTPie they lose JSON syntax highlighting because of this. If they were able to specify the prefix, however, that would be stripped, then they could see the JSON as they would see normal JSON via HTTPie. While I agree with you that this doesn't strengthen HTTPie's security posture, I think this serves to help its users of these types of APIs. |
@yashsriv Now I see what this is about — I have done a poor job reading the description the first time around. Thanks, @sigmavirus24. So I understand that What if we simply tweaked HTTPie's JSON handling so that when JSON is expected but the content is not valid, the parser would then try again while skipping anything before the first Do you have an example URL? |
@jkbrzt I don't have an example URL because I encountered this issue when I implemented XSSI protection on a REST api I'm working on for a private project. Also, I added the option to specify a prefix because I read online that some services use I'll try and find a public api on which you can test this. |
I'm interested in having this feature. Would like to see the feature merged! Is there anything which can be improved to get this done? |
httpie/output/formatters/json.py
Outdated
@@ -17,6 +17,10 @@ def format_body(self, body, mime): | |||
] | |||
if (self.kwargs['explicit_json'] or | |||
any(token in mime for token in maybe_json)): | |||
if 'xssi_prefix' in self.kwargs: | |||
token = self.kwargs["xssi_prefix"] | |||
if body[0:len(token)] == token: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it not be simpler to do:
if body.startswith(token):
It looks as though token
being ''
won't get through to here because otherwise you'll always find this condition to be true and you'll always make a copy.
That said, even if you specify the common value that @jkbrzt found, this should be a relatively cheap check (to use startswith).
Rebased and updated as per @sigmavirus24 's recommendation |
Is this waiting on anything else? |
I'm still having to fallback to |
Bump. Can we hope that this PR gets merged soon? |
httpie/cli.py
Outdated
@@ -168,6 +168,17 @@ def _split_lines(self, text, width): | |||
|
|||
""" | |||
) | |||
content_type.add_argument( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a content type. This feels like it should be with the output options.
httpie/output/formatters/json.py
Outdated
@@ -17,6 +17,10 @@ def format_body(self, body, mime): | |||
] | |||
if (self.kwargs['explicit_json'] or | |||
any(token in mime for token in maybe_json)): | |||
if 'xssi_prefix' in self.kwargs: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will always be true since you defaulted it in the cli arguments. I think it'd be better to reverse this logic a bit:
xssi_prefix = self.kwargs["xssi_prefix"]
if xssi_prefix and body.startswith(xssi_prefix):
body = body[len(xssi_prefix):]
XSSI Vulnerability protection usually involves prefixing all JSON data with a particular prefix to make it non-executable. HTTPie does not display the resulting JSON in pretty print format. My changes add support for specifying a prefix so that HTTPie can strip it before parsing the data as JSON.
XSSI Vulnerability protection usually involves prefixing all JSON
data with a particular prefix to make it non-executable.
HTTPie does not display the resulting JSON in pretty print format.
My changes add support for specifying a prefix so that HTTPie can strip
it before parsing the data as JSON.