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

Better way to set cookies and other repeatable headers in Javascript Http functions #2737

Closed
cesarvarela opened this issue Apr 25, 2018 · 16 comments · Fixed by #4317
Closed

Comments

@cesarvarela
Copy link

cesarvarela commented Apr 25, 2018

I'm kind of lost here, but I don't see anything related to using cookies in the docs.

I need to write/read them, is it possible?

@cesarvarela cesarvarela changed the title Send cookies in HttpTrigger Use cookies in HttpTrigger Apr 25, 2018
@paulbatum
Copy link
Member

please provide more information (see our issue template)

@cesarvarela
Copy link
Author

Look at this:
https://docs.microsoft.com/en-us/azure/azure-functions/functions-reference-node

It mentions headers, body, and status, but not cookies:

// Define a valid response object.
res = { status: 201, body: "Insert succeeded." };
context.done(null, res);   

@paulbatum paulbatum added this to the Active Questions milestone Apr 26, 2018
@paulbatum
Copy link
Member

Ahh, so you're trying to do it from JavaScript? I'm not sure if this is possible right now.

@mhoeger do you know?

@mhoeger
Copy link
Contributor

mhoeger commented Apr 26, 2018

@cesarvarela I think I responded to your stackoverflow post :) I recommended using the HTTP header "Set-Cookie". Is there anything special about the express "cookies" method that can't be done through HTTP headers? Or would you like to see this as a feature?

@cesarvarela
Copy link
Author

Yes you did.

That would work, but I think it would be nice to make it work like the other settings, something like:

// Define a valid response object.
res = { status: 201, cookies: { mycookie: 'text' },  body: "Insert succeeded." };
context.done(null, res);   

@mhoeger
Copy link
Contributor

mhoeger commented Apr 26, 2018

Sounds good. Thanks for the input! I'd imagine other developers coming from express will also expect a "cookies" property as is done there.

Instead of adding a cookies property, I'm inclined to stick to the HTTP standard way of doing things with Set-Cookie defined in the headers. A compromise might be to have a setCookie(serializedCookie: string) method on our res object to easily set cookies. We already have one such method for Content-Type. This setCookie method could be used with existing
npm packages like cookie to help serialize/parse cookies.

I'll re-triage this issue for now as a feature request for a setCookie() helper method, but input is very welcome!

@mhoeger mhoeger modified the milestones: Active Questions, Backlog Apr 26, 2018
@mhoeger mhoeger changed the title Use cookies in HttpTrigger Easier way to set cookies in Javascript Http functions Apr 26, 2018
@mhoeger mhoeger changed the title Easier way to set cookies in Javascript Http functions [Feature Request] Easier way to set cookies in Javascript Http functions Apr 26, 2018
@alanmartin
Copy link

The primary problem for me with the current approach is that it's quite difficult to set multiple cookies in one response.
I've found I have to add whitespace in the second header because javascript objects don't allow duplicate keys.


        context.res = {
            // status: 200, /* Defaults to 200 */
            body: "Hello " + (req.query.name || req.body.name),
            headers: {"Set-Cookie" : "name=value2", "Set-Cookie " : "name3=value5"}
        };

@maxhov
Copy link

maxhov commented Nov 3, 2018

I second @alanmartin. The current implementation is rather limited with regards to setting multiple cookies. Only with the mentioned work around, it is currently possible to set multiple cookies. The implementation suggested by @cesarvarela could be altered like this to facilitate multiple cookies:

...
res = { 
    status: 201, 
    cookies: [ 
        { 
            'name': 'cookie1',
            'value': value,
            'secure': true,
            'httponly': true,
            'path': '/',
            'domain': 'www.example.com',
            etc..
        },
        {
            'name': 'cookie2',
            'value': value,
            'secure': true,
            'httponly': true,
            'path': '/',
            'domain': 'www.example.com',
            etc..
       },
    ],
    body: "some body" 
}
...

@mhoeger mhoeger added bug and removed enhancement labels Nov 9, 2018
@mhoeger
Copy link
Contributor

mhoeger commented Nov 9, 2018

Good point, thanks for bringing this up! Will prioritize this as more than an enhancement. Note for others the current workaround is adding white spaces as @alanmartin suggested above.

@mhoeger mhoeger changed the title [Feature Request] Easier way to set cookies in Javascript Http functions Better way to set cookies and other repeatable headers in Javascript Http functions Nov 9, 2018
@securityvoid
Copy link

This is also impacting us as well. We're also looking to set multiple cookies.

@gthorvey
Copy link

gthorvey commented Mar 5, 2019

Good point, thanks for bringing this up! Will prioritize this as more than an enhancement. Note for others the current workaround is adding white spaces as @alanmartin suggested above.

I am not able to set multiple cookies correctly even with white spaces. I am getting the below error:
An unhandled host error has occurred.
Microsoft.AspNetCore.Server.Kestrel.Core: Invalid non-ASCII or control character in header: 0x0020.

@gpasq
Copy link

gpasq commented Mar 20, 2019

I get the same. Multiple cookies is required in many circumstances, and Azure simply can't handle them. How hard can this be to implement? (answer is "not that hard")

@mhoeger
Copy link
Contributor

mhoeger commented Apr 8, 2019

@gpasq - we always love and accept community contributions as an open source project :)

I am starting the work to enable setting multiple cookeis here FYI all on this thread: Azure/azure-functions-language-worker-protobuf#31

@mhoeger
Copy link
Contributor

mhoeger commented Jun 18, 2019

Update on this issue:
Although the fix is checked in, it has not yet been released. If you urgently need to set cookies, please add a proxy that modifies the response headers of your function as a workaround.

Here's an example:

image

Will update this issue when it is released.

@dcollien
Copy link

Checking up after 6m... was this ever released?

@mhoeger
Copy link
Contributor

mhoeger commented Dec 20, 2019

@dcollien Yes - sorry forgot to update the issue! You can set multiple cookies on a response object with the property "cookies", which takes an array of Cookie objects.

Ex:

context.res = {
    body: "Hello",
    status: 200,
    cookies: [
        {
            name: "cookie1",
            value: "value1"
        },
        {
            name: "cookie2",
            value: "value2",
            maxAge: 60 * 10
        }
    ]

I'll be sure to update the docs

@ghost ghost locked as resolved and limited conversation to collaborators Jan 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants