curlconverter icon indicating copy to clipboard operation
curlconverter copied to clipboard

Does not seems to parse correctly a PUT request

Open cderv opened this issue 7 years ago • 6 comments

Hi,

I am trying to use curlconverter to show an example about how to convert a curl command. The command is a PUT with data to send. the command is this one: curl -v -X PUT -H "X-IBM-Client-Secret:YOUR_CLIENT_SECRET" -H "X-IBM-Client-Id:YOUR_CLIENT_ID" -H "Content-Type: application/json" -d '{"clientName": "The Sample Outdoors Company", "redirectURIs": "https://example.com:5443", "ownerName": "John Smith", "ownerEmail": "[email protected]", "ownerCompany": "example.com", "ownerPhone": "555-123-4567"}' https://api.ibm.com/watsonanalytics/run/oauth2/v1/config

When I copy this command and run straighten, it seems not to be parsed correctly :

curl -v -X PUT -H "X-IBM-Client-Secret:YOUR_CLIENT_SECRET" -H "X-IBM-Client-Id:YOUR_CLIENT_ID" -H "Content-Type: application/json" -d '{"clientName": "The Sample Outdoors Company", "redirectURIs": "https://example.com:5443", "ownerName": "John Smith", "ownerEmail": "[email protected]", "ownerCompany": "example.com", "ownerPhone": "555-123-4567"}' https://api.ibm.com/watsonanalytics/run/oauth2/v1/config
[[1]]
$url
[1] "clientName"

$method
[1] "get"

$headers
$headers$`X-IBM-Client-Secret`
[1] "YOUR_CLIENT_SECRET"

$headers$`X-IBM-Client-Id`
[1] "YOUR_CLIENT_ID"

$headers$`Content-Type`
[1] "application/json"


$verbose
[1] TRUE

$url_parts
$scheme
NULL

$hostname
NULL

$port
NULL

$path
[1] "clientName"

$query
NULL

$params
NULL

$fragment
NULL

$username
NULL

$password
NULL

attr(,"class")
[1] "url"  "list"

$orig_curl
[1] "curl -v -X PUT -H \"X-IBM-Client-Secret:YOUR_CLIENT_SECRET\" -H \"X-IBM-Client-Id:YOUR_CLIENT_ID\" -H \"Content-Type: application/json\" -d '{\"clientName\": \"The Sample Outdoors Company\", \"redirectURIs\": \"https://example.com:5443\", \"ownerName\": \"John Smith\", \"ownerEmail\": \"[email protected]\", \"ownerCompany\": \"example.com\", \"ownerPhone\": \"555-123-4567\"}' https://api.ibm.com/watsonanalytics/run/oauth2/v1/config"

attr(,"class")
[1] "cc_obj" "list"  

attr(,"class")
[1] "cc_container" "list"      

I understand your package is base on the npm module NickCarneiro/curlconverter. When I use the test site provided https://curl.trillworks.com/#python to test my curl command, it seems it is parsed correctly in Python.

In the curl command, the -d part seems to cause problem, URL being at the end.

Is there something on my side ? Should we improve the JS function toR ? Can you help me to understand ?

thanks.

cderv avatar Jul 14 '17 08:07 cderv

I got the same if I use toPython in debug mode inside curlconverter:::process_url. It seems the node module in the package does not work as the node module in NickCarneiro/curlconverter

cat(.pkgenv$ct$call("curlconverter.toPython", x))
headers = {
    'X-IBM-Client-Secret': 'YOUR_CLIENT_SECRET',
    'X-IBM-Client-Id': 'YOUR_CLIENT_ID',
    'Content-Type': 'application/json',
}

requests.get('clientName', headers=headers)

cderv avatar Jul 14 '17 09:07 cderv

Just for you to know I tried, kind of brutally, to update npm modules to see if it is working. It is in a branch in my fork. Lots of change since 2 years. Python parsing is now working like expected. R parsing is now better (find PUT, and correct URL) but headers are not returned and I don't know why.

Not sure it is the correct way to go but npm module surely evolve and is better now..

cderv avatar Jul 14 '17 11:07 cderv

Thx for both checking the pkg out and the intensive debugging!

I've been poking at an [unpublic] version of the package that no longer relies on the V8 package (i.e. it will be C-backed vs js-backed). The package was removed from CRAN b/c of issues related to the V8 components and it never worked well on many Windows systems due to the way the underlying v8 C library was built and integrated (esp on 32-bit systems).

Let me see if I can do an interim patch to this, though (I've done it to provide support for other command-line components so this shouldn't be too bad).

On Fri, Jul 14, 2017 at 7:33 AM, Christophe Dervieux < [email protected]> wrote:

Just for you to know I tried, kind of brutally, to update npm modules to see if it is working. It is in a branch in my fork. Lots of change since 2 years. Python parsing is now working like expected. R parsing is now better (find PUT, and correct URL) but headers are not returned and I don't know why.

Not sure it is the correct way to go but npm module surely evolve and is better now..

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/hrbrmstr/curlconverter/issues/17#issuecomment-315338231, or mute the thread https://github.com/notifications/unsubscribe-auth/AAfHtmtaJ5HnUdJTsZJRNmWQkFvAu9edks5sN1IjgaJpZM4OX_q- .

hrbrmstr avatar Jul 14 '17 12:07 hrbrmstr

It looks like I just need to tweak the data handling a bit (I also need to fold back in some changes I had made for other issues). Will try to do it ASAP.

On Fri, Jul 14, 2017 at 8:58 AM, Bob Rudis [email protected] wrote:

Thx for both checking the pkg out and the intensive debugging!

I've been poking at an [unpublic] version of the package that no longer relies on the V8 package (i.e. it will be C-backed vs js-backed). The package was removed from CRAN b/c of issues related to the V8 components and it never worked well on many Windows systems due to the way the underlying v8 C library was built and integrated (esp on 32-bit systems).

Let me see if I can do an interim patch to this, though (I've done it to provide support for other command-line components so this shouldn't be too bad).

On Fri, Jul 14, 2017 at 7:33 AM, Christophe Dervieux < [email protected]> wrote:

Just for you to know I tried, kind of brutally, to update npm modules to see if it is working. It is in a branch in my fork. Lots of change since 2 years. Python parsing is now working like expected. R parsing is now better (find PUT, and correct URL) but headers are not returned and I don't know why.

Not sure it is the correct way to go but npm module surely evolve and is better now..

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/hrbrmstr/curlconverter/issues/17#issuecomment-315338231, or mute the thread https://github.com/notifications/unsubscribe-auth/AAfHtmtaJ5HnUdJTsZJRNmWQkFvAu9edks5sN1IjgaJpZM4OX_q- .

hrbrmstr avatar Jul 14 '17 13:07 hrbrmstr

Oh thanks! If I could help with something do not hesitate. Digging into the problem made me learn a lot on JS and V8. I did not manage to go after a point where V8 package did not manage to parse correctly the request return by toR function. Did not understand why. If a newer version relying on C will arrive, it is cool. Hope you don't keep it unpublic too long.

cderv avatar Jul 14 '17 15:07 cderv

aye. there's an issue I found (and failed to log here or in V8) where it does, indeed, fail to parse or half-parses many data elements (I use this pkg quite a bit when doing work-work and sometimes have to workaround it, which is also one reason I'm rewriting it).

Lemme try to clean up the C-backed one for early next week and i'll push a branch up you can take a look at. It's super helpful having other sets of eyes on this #ty

On Fri, Jul 14, 2017 at 11:10 AM, Christophe Dervieux < [email protected]> wrote:

Oh thanks! If I could help with something do not hesitate. Digging into the problem made me learn a lot on JS and V8. I did not manage to go after a point where V8 package did not manage to parse correctly the request return by toR function. Did not understand why. If a newer version relying on C will arrive, it is cool. Hope you don't keep it unpublic too long.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/hrbrmstr/curlconverter/issues/17#issuecomment-315383952, or mute the thread https://github.com/notifications/unsubscribe-auth/AAfHtmTgoSyAjOEvgwrHQDYGZ1tmViFHks5sN4RJgaJpZM4OX_q- .

hrbrmstr avatar Jul 14 '17 16:07 hrbrmstr