Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes several issues with local HTTP requests #30

Merged
merged 5 commits into from Oct 13, 2017

Conversation

idpaterson
Copy link
Collaborator

@idpaterson idpaterson commented Oct 12, 2017

From https://community.webcore.co/t/xml-response-handling-from-web-request/878/31

PR just for ease of seeing the changes; showing bits of code in Discourse was not working well for this one.

$httpContentType is null

The wc_async_reply event is expecting a contentType parameter but localHttpRequestHandler does not provide it. Added contentType: mediaType to the event data.

mediaType can include charset and boundary

mediaType in localHttpRequestHandler is just the Content-Type header; the charset needs to be removed (i.e. passing contentType: mediaType gives me a value of application/json; charset=utf-8.

def mediaType = hubResponse.getHeaders()['content-type']?.toLowerCase().tokenize(';')[0]

response.body is not available

The localHttpRequestHandler does not pass along the response body, only any json that is parsed from it. Instead, both the json and raw response body should be passed. Note that changes to wc_async_reply could also affect asyncHttpRequestHandler but it didn't look like anything would break.

Tweaked localHttpRequestHandler to pass responseData: data, jsonData: json then updated the event handler.

if (event.schedule.stack) {
  event.schedule.stack.response = event.responseData
  event.schedule.stack.json = event.jsonData
}

json response can't have leading or trailing whitespace

The startsWith and endsWith calls should operate on a trimmed value:

def trimmed = data.trim()
if (trimmed.startsWith('{') && trimmed.endsWith('}')) {
    json = (LinkedHashMap) new groovy.json.JsonSlurper().parseText(trimmed)
} else if (trimmed.startsWith('[') && trimmed.endsWith(']')) {
    json = (List) new groovy.json.JsonSlurper().parseText(trimmed)
} else {
    json = [:]
}

hubResponse.json may be simpler option here, but maybe not. It caused problems when I tried to parse JSON data that is not an array or object (e.g. "text" or 5 or null)

Is $response.data.* a thing?

I thought I remembered $response.data.* being used to access JSON values, like {"test": "test"} accessed as $response.data.test, but I think I was mistaken because I couldn't find any code that does it. If that does happen for normal requests this PR does not address it for locals, you would access the request with $response.test (which I think is the correct way anyway, just making sure).

@idpaterson
Copy link
Collaborator Author

Oh, and please carefully review these changes because I haven't done enough testing to guarantee that they're safe...

@idpaterson idpaterson force-pushed the pull-requests/local-http-requests branch from 50bff47 to c1f74cb Compare October 12, 2017 17:05
@ady624 ady624 merged commit 0d0632d into ady624:master Oct 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants