jonsamwell / angular-http-batcher

Enables HTTP batch request with AngularJS
MIT License
96 stars 28 forks source link

Fix error in trimJsonProtectionVulnerability when parse responses in JSON #26

Closed magarcia closed 9 years ago

magarcia commented 9 years ago

I have a server that use json for batch requests, as seen in example. When the response arrive to trimJsonProtectionVulnerability data is an array not an string, so the functions fails with a Object has no method 'replace'.

I think the solution I made is more appropriate, because only try to trim json protection if data is an string.

Example of expected request and response in my server (based on django-batch-requests)

Request:

[
  {
    "url" : "/views/",
    "method" : "get"
  },
  {
    "url" : "/views/",
    "method" : "post",
    "body" : "{\"text\": \"some text\"}",
    "headers" : {"Content-Type": "application/json"}
  }
]

Response:

[
    {
        "headers": {
            "Content-Type": "text/html; charset=utf-8"
        },
        "status_code": 200,
        "body": "Success!",
        "reason_phrase": "OK"
    },
    {
        "headers": {
            "Content-Type": "text/html; charset=utf-8"
        },
        "status_code": 201,
        "body": "{\"text\": \"some text\"}",
        "reason_phrase": "CREATED"
    }
]
jonsamwell commented 9 years ago

@magarcia Thanks for this. Yes your fix is much better! Any chance you can do a test for this functionality? Also please only change the actual source file and none of the dist file or package files as these are by the build.

Awesome work!!!

magarcia commented 9 years ago

Added tests and revert changes in dist and versions.

jonsamwell commented 9 years ago

Thanks for this, I'll get a release out sometime today.

jonsamwell commented 9 years ago

Hi @magarcia I wouldn't have thought the default httpAdapter would have worked it your data is already an object by the time it calls parse response on the adapter. Or are you using a custom adapter?

jonsamwell commented 9 years ago

This fix is now in the new release (v1.11.3).

magarcia commented 9 years ago

@jonsamwell I'm using a custom adapter. I have planned to publish the adapter when finish, but for now is very coupled to my app code.

jonsamwell commented 9 years ago

Awesome! What format does in serialise for out of interest?

magarcia commented 9 years ago

JSON like the examples I put in the first comment.

Request:

[
  {
    "url" : "/views/",
    "method" : "get"
  },
  {
    "url" : "/views/",
    "method" : "post",
    "body" : "{\"text\": \"some text\"}",
    "headers" : {"Content-Type": "application/json"}
  }
]

Response:

[
    {
        "headers": {
            "Content-Type": "text/html; charset=utf-8"
        },
        "status_code": 200,
        "body": "Success!",
        "reason_phrase": "OK"
    },
    {
        "headers": {
            "Content-Type": "text/html; charset=utf-8"
        },
        "status_code": 201,
        "body": "{\"text\": \"some text\"}",
        "reason_phrase": "CREATED"
    }
]
jonsamwell commented 9 years ago

Does the current node js adapter not do what you want?

https://github.com/jonsamwell/angular-http-batcher/blob/master/src/services/adapters/nodeJsMultiFetchAdapter.js

magarcia commented 9 years ago

Not, the node js adapter use get query parameters for make GET requests, but my backend server expect all the requests as a JSON object. In other hand, I had custom headers that changes dynamically so I update this headers in the adapter.