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

A cookie with a key but no value breaks a GET /functionname #88

Closed
rjbertsche opened this issue Apr 10, 2017 · 8 comments
Closed

A cookie with a key but no value breaks a GET /functionname #88

rjbertsche opened this issue Apr 10, 2017 · 8 comments

Comments

@rjbertsche
Copy link

rjbertsche commented Apr 10, 2017

using plumber (both latest release and the development version), we had an issue with a GET that'd return a 500. after some wireshark and a lot of wget iterations, this entry in my cookies.txt seems to have been the cause:

.domain.com TRUE / FALSE 1626207978 km_uq

which has a key but no value. compared to:

.domain.com TRUE / FALSE 1626131221 km_lv x

which works.

we were seeing that chrome (where we got these cookies) was failing but incognito mode wasn't failing. if there's anything further I can provide, just let me know.

@rjbertsche rjbertsche changed the title A Cookie with a key but no value breaks a GET / A Cookie with a key but no value breaks a GET Apr 10, 2017
@rjbertsche rjbertsche changed the title A Cookie with a key but no value breaks a GET A Cookie with a key but no value breaks a GET /functionname Apr 10, 2017
@rjbertsche rjbertsche changed the title A Cookie with a key but no value breaks a GET /functionname A cookie with a key but no value breaks a GET /functionname Apr 10, 2017
@rjbertsche
Copy link
Author

rjbertsche commented Apr 11, 2017

I forgot versions; R 3.2.0, plumber 0.3.1 and the github head

here's a counter we used for testing:

#* @get /counter
function(req, res){
  count <- 0
  if (!is.null(req$cookies$visitcounter)){
    count <- as.numeric(req$cookies$visitcounter)
  }
  res$setCookie("visitcounter", count+1)
  return(paste0("This is visit #", count))
}

@jen0liu
Copy link

jen0liu commented Apr 24, 2017

@trestletech Any comments on this issue? We are still using incognito window at this point.

@trestletech
Copy link
Contributor

trestletech commented May 3, 2017

Hey there. Thanks for reporting this.

I don't quite understand the format that you posted for the cookie in the initial example; can you clarify?

If you can send over the exact format of the cookie header that's failing (you can copy this out of the request headers in Chrome where things are failing) I should be able to dig in.

@rjbertsche
Copy link
Author

rjbertsche commented May 3, 2017

sure thing, thank you for the response. so i'm gonna use a cookies.txt format, the netscape style for this example. the field breakdown is as follows, (i'm grabbing from stackexchange here):

domain - The domain that created AND that can read the variable.

flag - A TRUE/FALSE value indicating if all machines within a given domain can access the variable. This value is set automatically by the browser, depending on the value you set for domain.

path - The path within the domain that the variable is valid for.

secure - A TRUE/FALSE value indicating if a secure connection with the domain is needed to access the variable.

expiration - The UNIX time that the variable will expire on. UNIX time is defined as the number of seconds since Jan 1, 1970 00:00:00 GMT.

name - The name of the variable.

value - The value of the variable.

here's a section of this exported cookies.txt format:

.domain.com TRUE / FALSE 1626207978 km_ai rjmb%40domain.com
.domain.com TRUE / FALSE 1626207978 km_ni rjmb%40domain.com
.domain.com TRUE / FALSE 1626207978 km_uq

if you had it in a csv-style, it'd look like this:

.domain.com,TRUE,/,FALSE,1626207978,km_ai,rjmb%40domain.com
.domain.com,TRUE,/,FALSE,1626207978,km_ni,rjmb%40domain.com
.domain.com,TRUE,/,FALSE,1626207978,km_uq,

so 'km_ai' and 'km_ni' are fine. 'km_uq' is not. I am not strong in R, but in https://github.com/trestletech/plumber/blob/master/R/cookie-parser.R it looks like you're splitting the names and cookies into two seperate values and then zipping them together again:

cookies <- lapply(cookieList, "[[", 2)
names(cookies) <- sapply(cookieList, "[[", 1)

if the names and values don't have the same number of members when you merge them, that seems to be a likely candidate to produce our error.

again, thanks for your time.

@trestletech
Copy link
Contributor

trestletech commented May 3, 2017

Totally. Yeah that's the code that I'm looking at.

The reason I ask for the header as it comes from Chrome is that I need to see the raw text that's being sent over in the header to understand where the parsing is failing. Do you mind copying and pasting the header out of the Chrome inspector? I'm not sure if an empty cookie will be cookiename= or just cookiename.

@rjbertsche
Copy link
Author

rjbertsche commented May 3, 2017

sure thing. the relevent section from above:

km_ai=rjmb%40domain.com; km_ni=rjmb%40domain.com; km_uq=; disable_welcomebox_v1=true;

the full text:

Cookie:km_lv=x; km_ai=rjmb%40domain.com; km_ni=rjmb%40domain.com; km_uq=; disable_welcomebox_v1=true; _hp2_id.3794337379=%7B%22userId%22%3A6763198385762388%2C%22pageviewId%22%3A%227139546965816785%22%2C%22sessionId%22%3A%224023884059878287%22%2C%22identity%22%3Anull%2C%22trackerVersion%22%3A%223.0%22%7D; __utma=246129028.429209940.1488841413.1491523169.1492723443.11; __utmc=246129028; __utmz=246129028.1488841413.1.1.utmcsr=(direct)|utmccn=(direct)|utmcmd=(none); __hstc=183755129.bd1e1fe80d7b806a8a06fd9b0bff3556.1488841416544.1491523170865.1492723447263.11; __hssrc=1; hubspotutk=bd1e1fe80d7b806a8a06fd9b0bff3556

@trestletech
Copy link
Contributor

trestletech commented May 13, 2017

Should be fixed on master now. Let me know if it's still giving you trouble.

@rjbertsche
Copy link
Author

rjbertsche commented May 16, 2017

works now, thanks for the update!

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

No branches or pull requests

3 participants