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

Order of Cookie is getting changed when response is returned from Lambda when set as target against ALB #1414

Closed
ElectricVampire opened this issue Jan 19, 2023 · 9 comments
Labels
bug This issue is a bug. module/aspnetcore-support p2 This is a standard priority issue queued

Comments

@ElectricVampire
Copy link

Describe the bug

In my code I am deleting the cookie before setting the new value for it.

// Code executed on 19th Jan
HttpContext.Response.Cookies.Delete("testCookie");
HttpContext.Response.Cookies.Append("testCookie", "test" , option);

Now I should see following in response in browser -

Set-Cookie  testCookie=; expires=Thu, 18 Jan 2023 18:33:48 GMT; path=/                        
Set-Cookie  testCookie=test; expires=Thu, 19 Jan 2023 18:33:48 GMT; path=/

This set testCookie to value test and returned in subsequent request.

Above is the behavior of my code when hosted on prem and EKS.

Now when hosted the code in lambda -
Now I see following in response in browser -

Set-Cookie  testCookie=test; expires=Thu, 19 Jan 2023 18:33:48 GMT; path=/
Set-Cookie  testCookie=; expires=Thu, 18 Jan 2023 18:33:48 GMT; path=/  

This set testCookie to value empty which is also expired and not returned in subsequent request.

https://github.com/aws/aws-lambda-dotnet/blob/master/Libraries/src/Amazon.Lambda.AspNetCoreServer/ApplicationLoadBalancerFunction.cs#L167

Here we are moving the headers from HeaderDictionary to IDictionary does this modify the order of cookies with same name?

Expected Behavior

I should see the cookie in response header in same order in which they were set.

Current Behavior

Order of cookie is getting changed

Reproduction Steps

Host an API in ALB and set delete the of cookie and then set the same value. You will issue getting reproduced.

Possible Solution

No response

Additional Information/Context

No response

AWS .NET SDK and/or Package version used

Amazon.Lambda.AspNetCoreServer.7.2.0

Targeted .NET Platform

.NET 6

Operating System and version

AmazonLinux - Lambda

@ElectricVampire ElectricVampire added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 19, 2023
@ashishdhingra
Copy link
Contributor

@ElectricVampire Thanks for reporting the issue. Could you please share the following:

  • Your use case, why not just set the cookie with new value and expiration rather than deleting it first.
  • Sample code solution which reproduces the issue, just want to make sure we have consistent repro.

Thanks,
Ashish

@ashishdhingra ashishdhingra added response-requested Waiting on additional info and feedback. Will move to close soon in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Jan 19, 2023
@ElectricVampire
Copy link
Author

ElectricVampire commented Jan 20, 2023

@ashishdhingra

  1. Our application is multi layer application and in request/response pipeline there are various component not owned by our team. One of such component clears all the cookies for their own business logic and security concerns. But we are up in the pipeline hence we add the latest value to cookie.
  2. I created code from Lambda Web API template and just added/deleted cookies in Values controller Get method to reproduce the issue. https://github.com/ElectricVampire/LambdaWebApi.

@ElectricVampire
Copy link
Author

ElectricVampire commented Jan 20, 2023

awselb/2.0 Headers in Browser : (Issue)
Set-Cookie testCookie=ABC;
Set-Cookie testCookie=; // This will clear the cookie as its after ABC
Set-Cookie dummy=abc;

Kestrel Headers in Browser (Expected)
Set-Cookie dummy=abc;
Set-Cookie testCookie=;
Set-Cookie testCookie=ABC;

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to close soon in 7 days. label Jan 21, 2023
@ashishdhingra ashishdhingra added needs-investigation p2 This is a standard priority issue queued labels Jan 27, 2023
@ashishdhingra
Copy link
Contributor

ashishdhingra commented Sep 24, 2024

Used instructions at Lambda functions as targets for Application Load Balancers to configure Serverless Web API Lambda function with ALB.

Customer provided code:
Controllers\ValuesController.cs

using Microsoft.AspNetCore.Mvc;

namespace LambdaWebApiALBTest.Controllers
{
    [Route("api/[controller]")]
    public class ValuesController : ControllerBase
    {
        private IHttpContextAccessor _httpContextAccessor;

        public ValuesController(IHttpContextAccessor httpContextAccessor)
        {
            _httpContextAccessor = httpContextAccessor;
        }

        // GET api/values
        [HttpGet]
        public IEnumerable<string> Get()
        {
            var response = _httpContextAccessor.HttpContext.Response;
            response.Cookies.Append("dummy", "abc");
            response.Cookies.Delete("testCookie");
            response.Cookies.Append("testCookie", "ABC");

            return new string[] { "value1", "value2" };
        }

        // GET api/values/5
        [HttpGet("{id}")]
        public string Get(int id)
        {
            return "value";
        }

        // POST api/values
        [HttpPost]
        public void Post([FromBody] string value)
        {
        }

        // PUT api/values/5
        [HttpPut("{id}")]
        public void Put(int id, [FromBody] string value)
        {
        }

        // DELETE api/values/5
        [HttpDelete("{id}")]
        public void Delete(int id)
        {
        }
    }
}

Startup.cs

...
        public IConfiguration Configuration { get; }

        // This method gets called by the runtime. Use this method to add services to the container
        public void ConfigureServices(IServiceCollection services)
        {
            services.AddControllers();
            services.AddHttpContextAccessor();
        }
...

Findings:

The only workaround for such scenarios is to build a generic wrapper around System.Collections.Specialized.OrderedDictionary.

OrderedDictionary<TKey, TValue>

Code taken from here

using System;
using System.Collections;
using System.Collections.Generic;
using System.Collections.Specialized;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Amazon.Lambda.AspNetCoreServer.Internal
{
    internal class OrderedDictionary<TKey, TValue> : IDictionary<TKey, TValue>, IDictionary
    {
        OrderedDictionary privateDictionary;

        public OrderedDictionary()
        {
            this.privateDictionary = new OrderedDictionary();
        }

        public OrderedDictionary(IDictionary<TKey, TValue> dictionary)
        {
            if (dictionary != null)
            {
                this.privateDictionary = new OrderedDictionary();

                foreach (KeyValuePair<TKey, TValue> pair in dictionary)
                {
                    this.privateDictionary.Add(pair.Key, pair.Value);
                }
            }
        }

        public int Count
        {
            get
            {
                return this.privateDictionary.Count;
            }
        }

        public bool IsReadOnly
        {
            get
            {
                return false;
            }
        }

        public TValue this[TKey key]
        {
            get
            {
                if (key == null)
                {
                    throw new ArgumentNullException(nameof(key));
                }

                if (this.privateDictionary.Contains(key))
                {
                    return (TValue)this.privateDictionary[(object)key];
                }
                else
                {
                    throw new KeyNotFoundException();
                }
            }
            set
            {
                if (key == null)
                {
                    throw new ArgumentNullException(nameof(key));
                }

                this.privateDictionary[(object)key] = value;
            }
        }

        public ICollection<TKey> Keys
        {
            get
            {
                List<TKey> keys = new List<TKey>(this.privateDictionary.Count);

                foreach (TKey key in this.privateDictionary.Keys)
                {
                    keys.Add(key);
                }

                // Keys should be put in a ReadOnlyCollection,
                // but since this is an internal class, for performance reasons,
                // we choose to avoid creating yet another collection.

                return keys;
            }
        }

        public ICollection<TValue> Values
        {
            get
            {
                List<TValue> values = new List<TValue>(this.privateDictionary.Count);

                foreach (TValue value in this.privateDictionary.Values)
                {
                    values.Add(value);
                }

                // Values should be put in a ReadOnlyCollection,
                // but since this is an internal class, for performance reasons,
                // we choose to avoid creating yet another collection.

                return values;
            }
        }

        public void Add(KeyValuePair<TKey, TValue> item)
        {
            Add(item.Key, item.Value);
        }

        public void Add(TKey key, TValue value)
        {
            if (key == null)
            {
                throw new ArgumentNullException(nameof(key));
            }

            this.privateDictionary.Add(key, value);
        }

        public void Clear()
        {
            this.privateDictionary.Clear();
        }

        public bool Contains(KeyValuePair<TKey, TValue> item)
        {
            if (item.Key == null || !this.privateDictionary.Contains(item.Key))
            {
                return false;
            }
            else
            {
                return this.privateDictionary[(object)item.Key].Equals(item.Value);
            }
        }

        public bool ContainsKey(TKey key)
        {
            if (key == null)
            {
                throw new ArgumentNullException(nameof(key));
            }

            return this.privateDictionary.Contains(key);
        }

        public void CopyTo(KeyValuePair<TKey, TValue>[] array, int arrayIndex)
        {
            if (array == null)
            {
                throw new ArgumentNullException("array");
            }

            if (arrayIndex < 0)
            {
                throw new ArgumentOutOfRangeException("arrayIndex");
            }

            if (array.Rank > 1 || arrayIndex >= array.Length || array.Length - arrayIndex < this.privateDictionary.Count)
            {
                throw new ArgumentException("Bad destination array.", nameof(array));
            }

            int index = arrayIndex;
            foreach (DictionaryEntry entry in this.privateDictionary)
            {
                array[index] = new KeyValuePair<TKey, TValue>((TKey)entry.Key, (TValue)entry.Value);
                index++;
            }
        }

        public IEnumerator<KeyValuePair<TKey, TValue>> GetEnumerator()
        {
            foreach (DictionaryEntry entry in this.privateDictionary)
            {
                yield return new KeyValuePair<TKey, TValue>((TKey)entry.Key, (TValue)entry.Value);
            }
        }

        IEnumerator IEnumerable.GetEnumerator()
        {
            return GetEnumerator();
        }

        public bool Remove(KeyValuePair<TKey, TValue> item)
        {
            if (Contains(item))
            {
                this.privateDictionary.Remove(item.Key);
                return true;
            }
            else
            {
                return false;
            }
        }

        public bool Remove(TKey key)
        {
            if (key == null)
            {
                throw new ArgumentNullException(nameof(key));
            }

            if (this.privateDictionary.Contains(key))
            {
                this.privateDictionary.Remove(key);
                return true;
            }
            else
            {
                return false;
            }
        }

        public bool TryGetValue(TKey key, out TValue value)
        {
            if (key == null)
            {
                throw new ArgumentNullException(nameof(key));
            }

            bool keyExists = this.privateDictionary.Contains(key);
            value = keyExists ? (TValue)this.privateDictionary[(object)key] : default(TValue);

            return keyExists;
        }

        void IDictionary.Add(object key, object value)
        {
            this.privateDictionary.Add(key, value);
        }

        void IDictionary.Clear()
        {
            this.privateDictionary.Clear();
        }

        bool IDictionary.Contains(object key)
        {
            return this.privateDictionary.Contains(key);
        }

        IDictionaryEnumerator IDictionary.GetEnumerator()
        {
            return this.privateDictionary.GetEnumerator();
        }

        bool IDictionary.IsFixedSize
        {
            get
            {
                return ((IDictionary)this.privateDictionary).IsFixedSize;
            }
        }

        bool IDictionary.IsReadOnly
        {
            get
            {
                return this.privateDictionary.IsReadOnly;
            }
        }

        ICollection IDictionary.Keys
        {
            get
            {
                return this.privateDictionary.Keys;
            }
        }

        void IDictionary.Remove(object key)
        {
            this.privateDictionary.Remove(key);
        }

        ICollection IDictionary.Values
        {
            get
            {
                return this.privateDictionary.Values;
            }
        }

        object IDictionary.this[object key]
        {
            get
            {
                return this.privateDictionary[key];
            }
            set
            {
                this.privateDictionary[key] = value;
            }
        }

        void ICollection.CopyTo(Array array, int index)
        {
            this.privateDictionary.CopyTo(array, index);
        }

        int ICollection.Count
        {
            get
            {
                return this.privateDictionary.Count;
            }
        }

        bool ICollection.IsSynchronized
        {
            get
            {
                return ((ICollection)this.privateDictionary).IsSynchronized;
            }
        }

        object ICollection.SyncRoot
        {
            get
            {
                return ((ICollection)this.privateDictionary).SyncRoot;
            }
        }
    }
}

EDIT:
Based on testing by adding _logger.LogInformation($"MultiHeaderValuesEnabled: {this._multiHeaderValuesEnabled}"); following is logged in CloudWatch logs:

2024-09-24T23:02:26.898Z	b453ae4a-04b5-42f8-8309-6775fb01d7a6	[Information] Amazon.Lambda.AspNetCoreServer.AbstractAspNetCoreFunction: MultiHeaderValuesEnabled: False

Since this._multiHeaderValuesEnabled is false, we are setting response.Headers[kvp.Key] = kvp.Value[0] to first value here. Hence other Set-Cookie values are lost.

To allow Set-Cookie multi-value headers, Multi value headers needs to be enabled for ALB target group; refer section Responses with multi-value headers. Once this is enabled, ALB makes use of multiValueHeaders collection, instead of headers collection.

Order of cookies doesn't appear to be getting changed in ApplicationLoadBalancerResponse MarshallResponse() since adding below code:

_logger.LogInformation($"ResponseFeatures HEADERS: {string.Join('|', (responseFeatures.Headers.Select(kv => kv.Key + ": " + kv.Value)))}");
_logger.LogInformation($"Multi-value HEADERS: {string.Join('|', (response.MultiValueHeaders?.Select(kv => kv.Key + ": " + "[" + string.Join('|', kv.Value) + "]")))}");

emits below in the CloudWatch log:

2024-09-25T17:33:59.360Z	436e7411-fc5a-451a-9082-fbe4b0059c87	[Information] Amazon.Lambda.AspNetCoreServer.AbstractAspNetCoreFunction: ResponseFeatures HEADERS: Set-Cookie: dummy=abc; path=/,testCookie=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/,testCookie=ABC; path=/|Content-Type: application/json; charset=utf-8 

2024-09-25T17:33:59.360Z	436e7411-fc5a-451a-9082-fbe4b0059c87	[Information] Amazon.Lambda.AspNetCoreServer.AbstractAspNetCoreFunction: Multi-value HEADERS: Set-Cookie: [dummy=abc; path=/|testCookie=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/|testCookie=ABC; path=/]|Content-Type: [application/json; charset=utf-8] 

In the browser, order appears to be reversed:
Screenshot 2024-09-25 at 10 44 40 AM

Calling Reverse() fixes the issue, but that doesn't explain why browser received cookies in reverse order:

if (kvp.Key.Equals("Set-Cookie", StringComparison.OrdinalIgnoreCase))
    response.MultiValueHeaders[kvp.Key] = response.MultiValueHeaders[kvp.Key].Reverse().ToList();

There is a below note at Responses with multi-value headers:

The load balancer might send the headers to the client in a different order than the order specified in the Lambda response payload. Therefore, do not count on headers being returned in a specific order.

@ashishdhingra
Copy link
Contributor

ashishdhingra commented Sep 30, 2024

When testing against Amazon.Lambda.AspNetCoreServer.APIGatewayProxyFunction (set as the base class in LambdaEntryPoint), the order of setting cookies is below in ValuesController:

...
        // GET api/values
        [HttpGet]
        public IEnumerable<string> Get()
        {
            var response = _httpContextAccessor.HttpContext.Response;
            response.Cookies.Append("dummy", "abc");
            response.Cookies.Delete("testCookie");
            response.Cookies.Append("testCookie", "ABC");

            return new string[] { "value1", "value2" };
        }
...

The order of cookie as viewed in browser developer tools is below:
Screenshot 2024-09-30 at 3 33 29 PM

@ashishdhingra
Copy link
Contributor

Internal ticket with service team: P159349026

@ashishdhingra
Copy link
Contributor

@ElectricVampire The issue was reproducible using your code base where the order of cookie was getting reversed. It works fine when using RestApi.

However, I noticed the note at Responses with multi-value headers:

The load balancer might send the headers to the client in a different order than the order specified in the Lambda response payload. Therefore, do not count on headers being returned in a specific order.

So we should not be relying on the cookie order for ALB and most likely this behavior would not be changed from the service team.

I have opened a ticket with ALB service team for their inputs and would post any updates here.

@ashishdhingra
Copy link
Contributor

@ElectricVampire As noted in previous comment, there is a below note at Responses with multi-value headers:

The load balancer might send the headers to the client in a different order than the order specified in the Lambda response payload. Therefore, do not count on headers being returned in a specific order.

So we should not be relying on the cookie order for ALB. Although I have opened a ticket with ALB team and awaiting response, most likely this behavior would not be changed. Hence, closing this ticket since the behavior is controlled by ALB environment. I will post updates, if any, as I receive from the ALB team.

Thanks,
Ashish

@ashishdhingra ashishdhingra closed this as not planned Won't fix, can't repro, duplicate, stale Oct 10, 2024
Copy link
Contributor

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. module/aspnetcore-support p2 This is a standard priority issue queued
Projects
None yet
Development

No branches or pull requests

2 participants