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

Fix content-type "application/x-www-form-urlencoded" failed #184

Merged
merged 2 commits into from
Apr 13, 2022

Conversation

jonekeat
Copy link
Contributor

fixes #183

@artemklevtsov artemklevtsov changed the base branch from master to dev March 31, 2022 09:42
@artemklevtsov
Copy link
Collaborator

Thank you for the request.
Can you add test for the application/x-www-form-urlencoded case?

@jonekeat
Copy link
Contributor Author

jonekeat commented Mar 31, 2022

@artemklevtsov I have added a test case for POST method with content_type = "application/x-www-form-urlencoded", but there is 1 thing I quite confused:

I dont know why the handler cant extract the params from POST body in .req$parameters_body?

Inside the handler, the body is correct:

> request$body
[1] "foo=bar&a=b"

but the parameters_body is empty

> request$parameters_body
list()

@artemklevtsov
Copy link
Collaborator

See example https://github.com/rexyai/RestRserve/blob/master/inst/tinytest/test-cl-request.R#L91-L103
Try request with c("foo" = "bar", "a" = "b") body.

@jonekeat
Copy link
Contributor Author

See example https://github.com/rexyai/RestRserve/blob/master/inst/tinytest/test-cl-request.R#L91-L103 Try request with c("foo" = "bar", "a" = "b") body.

Weird, I still did not get the request$parameters_body, is the request object must passed to backend$set_request(r, headers = h, body = b) to run something like parse_params_body?

@artemklevtsov
Copy link
Collaborator

Parsing an url-encoded body defined here:

parse_form_urlencoded = function(body, request) {
if (length(body) > 0L) {
# Named character vector. Body parameters key-value pairs.
# Omit empty keys and empty values
body = body[nzchar(names(body)) & nzchar(body)]
request$parameters_body = as.list(body)
keys = names(body)
values = paste(cpp_url_encode(keys), cpp_url_encode(body), sep = "=", collapse = "&")
body = charToRaw(values)
} else {
body = raw()
}
request$body = body
return(request)
},

So we simply convert named vector to list, encode it and convert to raw-vector.

@jonekeat
Copy link
Contributor Author

jonekeat commented Apr 1, 2022

I see, that makes sense now. because Application$process_request() never pass the request to BackendRserve (so far I didnt saw that, let me know if i am wrong), so when use Application$process_request() the request$parameters_body return list()

  1. When we start the backend, and user send API request, do it pass the request directly to BackendRserve? or it will construct a RestRserve::Request object first then pass to backend?

  2. if the body is parsed correctly in the production setting (with backend serving), do we still need to include these parse_form_urlencoded & other parse functions in Application, just for the sake of testing?

@codecov
Copy link

codecov bot commented Apr 13, 2022

Codecov Report

Merging #184 (7553d7a) into dev (967c48c) will increase coverage by 0.07%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev     #184      +/-   ##
==========================================
+ Coverage   95.01%   95.09%   +0.07%     
==========================================
  Files          27       27              
  Lines        1305     1305              
==========================================
+ Hits         1240     1241       +1     
+ Misses         65       64       -1     
Impacted Files Coverage Δ
R/ContentHandlersFactory.R 97.05% <100.00%> (+1.47%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 967c48c...7553d7a. Read the comment docs.

@dselivanov
Copy link
Collaborator

  1. When we start the backend, and user send API request, do it pass the request directly to BackendRserve? or it will construct a RestRserve::Request object first then pass to backend?

HTTP request is initially processed by backend (Rserve) and transformed into R object (which is specific per backend). Than this R object is transformed into standard RestRserve::Request and then all operations happen with Request object.
And on the way back RestRserve::Response is converted to back-end-specific response.

if the body is parsed correctly in the production setting (with backend serving), do we still need to include these parse_form_urlencoded & other parse functions in Application, just for the sake of testing?

parse_form_urlencoded used internally, no need to call it the application

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.

[BUG] POST request with content-type "application/x-www-form-urlencoded" failed
3 participants