jonsamwell / angular-http-batcher

Enables HTTP batch request with AngularJS
MIT License
96 stars 28 forks source link

Batcher not sending cookies with each request. #14

Closed sarg3nt closed 9 years ago

sarg3nt commented 9 years ago

We are loving the http batcher but have found that it breaks our login system as it is not passing the cookies along with each header in the POST call to /api/batch. The main headers for the overall POST have the cookies, but not the "wrapped" ones. Example: Here are the actual POST requests headers:

Accept
application/json, text/plain, / Accept-Encoding gzip, deflate Accept-Language en-us Content-Length
968 Content-Type
multipart/mixed; charset=UTF-8; boundary=1429288092518 Cookie
EktGUID=cc34ebb5-bde2-4f7c-b53d-586da408456d; EkAnalytics=0; sellang=1033; utma=267318028.446158527 .1429210407.1429210407.1429222065.2; utmz=267318028.1429210407.1.1.utmcsr=(direct)|utmccn=(direct) |utmcmd=(none); _pk_id.2.6041=2cdce7d65e53ff7b.1429210407.2.1429222128.1429210600.; resolution=1920; BC_BANDWIDTH=1429225576812X2592; ecm=user_id=10040&AuthenticationToken=dbfa7c741fb941f29df99767657582e0 &site_id=/,439052712&userfullname=blakbarn%40selinc.com&displayname=blakbarn%40selinc.com&username=blakbarn %40selinc.com&new_site=/&unique_id=439052712&editoroptions=contentdesigner&site_preview=0&langvalue= &isMembershipUser=1&last_login_date=4/17/2015 9:15:21 AM&dm=.ad.selinc.com&DefaultLanguage=1033&NavLanguage =1033&SiteLanguage=1033&DefaultCurrency=840&SiteCurrency=840&UserCulture=1033&LastValidLanguageID=1033 &ekTimeZone=Pacific Standard Time; currentuser=%7B%22id%22%3A10040%2C%22authenticated%22%3Atrue%2C%22userName %22%3A%22blakbarn%40selinc.com%22%2C%22authToken%22%3A%22dbfa7c741fb941f29df99767657582e0%22%2C%22groups %22%3A%5B888888%2C98184%2C98046%2C98037%2C97812%2C97784%2C97713%2C664%2C396%2C384%2C138%5D%2C%22persistent %22%3Afalse%7D; .ASPXAUTH=F2DC9511D56C1045A6320CEF7DAC67462B8FD161747D8F4A41FDBF023E5F758557FE67F068 C8B1C6139152A2E351FDC01B8517C56BB80DD72EA06452B52FD923B535F9844139C8C656E75FD138DD52E611E10D435F94A9514D8A1B4706FF733FDFBE41AC2C8B45E81220F497311963AA3AC7D6B7EEB645398B26970C7DF7C5BAC6A419D6 ; SelincSession=blakbarn@selinc.com Host
davesargpc.ad.selinc.com Preview-Mode
false Referer https://davesargpc.ad.selinc.com/utils/elmah User-Agent
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:37.0) Gecko/20100101 Firefox/37.0

And here is the POST data itself, notice no cookies: --1429288092518 Content-Type: application/http; msgtype=request

GET /api/elmah/hosts/ HTTP/1.1 Host: davesargpc.ad.selinc.com Accept: application/json, text/plain, / Accept-Language: en-us Preview-Mode: false

--1429288092518 Content-Type: application/http; msgtype=request

GET /api/elmah/types/ HTTP/1.1 Host: davesargpc.ad.selinc.com Accept: application/json, text/plain, / Accept-Language: en-us Preview-Mode: false

--1429288092518 Content-Type: application/http; msgtype=request

GET /api/elmah/statuscodes/ HTTP/1.1 Host: davesargpc.ad.selinc.com Accept: application/json, text/plain, / Accept-Language: en-us Preview-Mode: false

--1429288092518 Content-Type: application/http; msgtype=request

GET /api/elmah/?filterCols=&filters=&page=1&sortCol=&sortDirection=asc HTTP/1.1 Host: davesargpc.ad.selinc.com Accept: application/json, text/plain, / Accept-Language: en-us Preview-Mode: false

--1429288092518--

Is this a bug or a config issue? We are using Web API 2

jonsamwell commented 9 years ago

Hmmm, this is an interesting one. I'm not sure what the spec says about sending cookies with every sub request. The cookies are added to the post automatically by the browser and it seems silly to send large cookies up with every sub request as this would dramatically increase the size of the message. However, it does make sense to send them from an atomic point of view. I think the way the web api http batcher handler is implemented is that is make a sub request for each item in the batch.

I think we should look at two approaches here:

1) See if I can send cookies with each request in the batch if needed. 2) See if we can modify the web api http batch handler to append the cookie is receives on the parent request to all sub requests. This would reduce the size of the request from the client.

Let me know what you think.

Jon

sarg3nt commented 9 years ago

I'm with you on it feeling like a waste and would love to see if we can get #2 working, that seems ideal, but if we can't then it would probably have to be #1.

sarg3nt commented 9 years ago

Any update on this?

sarg3nt commented 9 years ago

Hi Jon, we have only a few weeks left in our project and this one is a major bug for us right now Is there an estimate as to when you will have some time to work on this?
Thank you!

jonsamwell commented 9 years ago

I'm looking at it today - sorry I have neglected this I have been so busy! A few quick questions:

1) Are you cookies HTTPOnly? 2) Do you need to send cookies on CORS requests?

Thanks.

sarg3nt commented 9 years ago

No CORS for us. I'm not sure what you mean by Http only. All traffic is https if that's what you mean.

Sent from my iPhone

On May 28, 2015, at 4:17 PM, Jon Samwell notifications@github.com wrote:

I'm looking at it today - sorry I have neglected this I have been so busy! A few quick questions:

1) Are you cookies HTTPOnly? 2) Do you need to send cookies on CORS requests?

Thanks.

— Reply to this email directly or view it on GitHub.

jonsamwell commented 9 years ago

OK, if you don't know about it they more than likely aren't! So the new release v1.8.0 have code to include cookie on each segment of the batch request. You will need to enable this by setting 'sendCookies' to true on the config object for each batch endpoint.

    app.config([
            'httpBatchConfigProvider',
            function (httpBatchConfigProvider) {
                httpBatchConfigProvider.setAllowedBatchEndpoint(
                        'http://localhost:8080',
                        'http://localhost:8080/api/batch', {
                            batchRequestCollectionDelay: 500,
                            minimumBatchSize: 2,
                            sendCookies: true
                        });
            }
    ]);

If this doesn't work you have HTTPOnly cookie so we will need to figure out a server solution to this such as appending the cookies to each sub request as we mentioned above. Let me know how this goes.

Sorry again for the delay!

jonsamwell commented 9 years ago

@dsargent3220 did this work for you?

sarg3nt commented 9 years ago

Jon, sorry I hadn't gotten back to you on this. No it didn't as it turns out we did have HTTPOnly cookies, so we went with the server route instead. My coworker took that on and got it working. Here's the code we used. This is the default auto batch code from web API with the exception of the changes as noted in the comments (around line 53)

namespace System.Web.Http.Batch
{
    using System.Collections.Generic;
    using System.Diagnostics.CodeAnalysis;
    using System.Net.Http;
    using System.Threading;
    using System.Threading.Tasks;

    /// <summary>
    /// Default implementation of <see cref="HttpBatchHandler"/> that encodes the HTTP request/response messages as MIME multipart. /// 
    /// </summary>
    /// <remarks>
    /// By default, it buffers the HTTP request messages in memory during parsing.
    /// </remarks>
    public class SelHttpBatchHandler : DefaultHttpBatchHandler
    {
        /// <summary>
        /// Initializes a new instance of the <see cref="SelHttpBatchHandler"/> class.      /// 
        /// </summary>
        /// <param name="httpServer">The <see cref="HttpServer"/> for handling the individual batch requests.</param>
        public SelHttpBatchHandler(HttpServer httpServer)
            : base(httpServer)
        {
            this.ExecutionOrder = BatchExecutionOrder.Sequential;
        }

        /// <summary>
        /// Converts the incoming batch request into a collection of request messages.
        /// Override so we can copy the header information from the batch request in to each inner request.
        /// </summary>
        /// <param name="request">The request containing the batch request messages.</param>
        /// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
        /// <returns>A collection of <see cref="HttpRequestMessage"/>.</returns>
        [SuppressMessage("Microsoft.Design", "CA1006:DoNotNestGenericTypesInMemberSignatures", Justification = "We need to return a collection of request messages asynchronously.")]
        public override async Task<IList<HttpRequestMessage>> ParseBatchRequestsAsync(HttpRequestMessage request, CancellationToken cancellationToken)
        {
            if (request == null)
            {
                throw new ArgumentNullException("request");
            }

            List<HttpRequestMessage> requests = new List<HttpRequestMessage>();
            cancellationToken.ThrowIfCancellationRequested();
            MultipartStreamProvider streamProvider = await request.Content.ReadAsMultipartAsync();
            foreach (HttpContent httpContent in streamProvider.Contents)
            {
                cancellationToken.ThrowIfCancellationRequested();
                HttpRequestMessage innerRequest = await httpContent.ReadAsHttpRequestMessageAsync();

                // This is different from DefaultHttpBatchHandler
                // Next three lines are custom (Blake Barnett).
                // Gets the request cookie information and passes it to each inner request
                IEnumerable<string> cookie = null;
                if (request.Headers.TryGetValues("Cookie", out cookie))
                {
                    innerRequest.Headers.Add("Cookie", cookie);
                }

                // Get properties from Batch request
                innerRequest.CopyBatchRequestProperties(request);
                requests.Add(innerRequest);
            }

            return requests;
        }
    }
}

You would then use this the same way you would the default batch handler in the web api config section.

    config.Routes.MapHttpBatchRoute(
    routeName: "batch",
    routeTemplate: "api/batch",
    batchHandler: new SelHttpBatchHandler(GlobalConfiguration.DefaultServer));

Hope that helps.

jonsamwell commented 9 years ago

Thanks for this I'll add it to the read me in case anyone else has the same issue.