iri
iri copied to clipboard
iotaledger
Allow IXI Modules to dictate response's content-type.
Description
This PR allows IXI Modules to decide the content-type they wish to send to the client. The idea is that clients might want to, for example, load web content directly on their browser, get data in XML format, images, or whatever the IXI module serves.
This PR was created for issue https://github.com/iotaledger/iri/issues/615
Checklist:
- [x] My code follows the style guidelines for this project
- [X] I have performed a self-review of my own code
- [X] I have commented my code, particularly in hard-to-understand areas
- [X] New and existing unit tests pass locally with my changes
To test the IXI part, you can take https://github.com/brunoamancio/TangleNet , an IXI module which provides the client with web content from the Tangle.
Just query from the browser (get request): http://localhost:14265/?command=TangleNet.view&tail= parameter tail = Tail transaction of bundle with web content
Example:
It might also be possible to use gson to convert the query parameters to json. That way I can save some code. But for now I haven't tried it yet. I can analyse the possibility to improve that. (DONE)
@brunoamancio Pretty cool mate! What do you see as the end-user application? When would someone request web pages from the network?
@footloosejava With these changes, accessing tangle-only and hybrid-hosted (tangle/internet) web content will be a reality. I have already implemented a simple IXI module (https://github.com/brunoamancio/TangleNet) to allow nodes provide that to clients sending requests directly from their browsers.
I plan on developing it further to make this process as user-friendly as possible.
I will refactor the command logic in another PR. Until then, this PR will be left as is. UPDATE: I've seen @GalRogozinski's comment in the command-related PR (https://github.com/iotaledger/iri/pull/392#issuecomment-388640445). I'm also waiting for a response.
@brunoamancio I'll review this shortly. a few remarks:
-
have you tested backward compatibility with existing IXI modules? examples: https://github.com/bluedigits/nstats https://github.com/iotaledger/Snapshot.ixi https://github.com/iotaledger/MAM.ixi
-
adding
disableHtmlEscaping()is alarming, could you elaborate on the need and how do you mitigate attacks against input validation? for example, reflective XSS via"COMMAND " + command + " is not available on this node"? (I'll be looking into this as well) -
the
iotaAPIHeaderDefined()additions are too much to swallow IMHO. the idea is to make IRI more readable and maintainable, there must be a better way to do that. I understand the reason is to not make this PR more complex, but in that case, we should first change the command routing (a la #392) and then redactor this PR. - i.e. use this as a PoC, but not merge it just yet.
Answering @alon-e questions:
1 - In case the IXIResponse does not have the specific fields reserved for specifying the content and content-type of the response, the behavior is exactly as before.
That means, only if code is like the following will this feature be used:
... javascript code of IXI module
return Response.create({ contentType: "specific-content/type", content : "response in the response content-type format"})
...
That is done in the new method convertResponseToClientFormat in API.java
Also, the method getResponseContentType makes sure the default content-type is json, like it is today. Only if it is explicitly defined by the IXIModule (in its response), will the response type be changed.
Anyway, I can run those and tell you the results in a few days as well.
2 - The reason is that html content (and any other content format) is converted from the javascript-generated response from the IXI Module to IXIResponse using gson, which makes a desired response with html to be unreadable by the IRI client, since it's escaped.
More details on gson in the API.java (where escaping is disabled):
-
When gson is used to convert query parameter strings to json, the reason is to match the post request body structure, which is a string with json content. I'll discuss this in the respective point below, since any issues should be handled the same way.
-
When the request body has unescaped values, the only case where reflective XSS is "possible" is the example you gave: ErrorResponse.create("Command [" + command + "] is unknown"). Other usages of parameters always cast/convert to specific types before using values. In this case, it is already possible (in the current code) to send in whatever one wants to in the post request body. But, since it is only used to send an error message back to the client who sent the request, there is no issue. If it was an SQL query, for example, that would be a huge problem, but the java byte code itself can't be injected. Although, a way to prevent that, if there was an issue. The open source OWASP Enterprise Security API:
String safe = ESAPI.encoder().encodeForHTML( command ); -
When gson is used to convert a response to client format (method convertResponseToClientFormat(...)) there is no issue, since the client will get the response content in the expected format and knows how to handle it. It's just a conveyor belt from the IXI Module in this case.
There are no other usages of gson in the API.java class
*Alternative: It would be possible to explicitly unescape IXIResponses before sending it to the client. But that would mean IRI knows what should be abstract to it (the content-type).
3 - I completely agree with that. I don't currently have time for the refactoring of the command logic, but if nothing is committed until I have time, I will do it myself in a separate PR and after the merge this PR can be updated with the better structure.
Please don't delete this comment Note to myself: In class IXIResponse methods getContent() and getResponseContentType(): Ensure variables are empty when properties "content" and "contentType" are not strings in IXI Module (they might be object or number).
Hi @GalRogozinski . I have not, yet. I'm really busy at work (.NET developer here) these last weeks and didn't have much free time after working hours either.
Anyway, I will perform those tests asap, possibly this weekend.
Update: I'm working on Hercules, so I can't implement that. it'd be great if someone else could :)
Hi everyone, I'm interested in this topic. I opened a new PR (#1385) with some little improvements to keep discussing about it and I hope we could be able to eventually implement this feature.
