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

1.4 breaks custom headers on PUT HTTP requests #834

Closed
dmunch opened this issue Mar 27, 2017 · 6 comments
Closed

1.4 breaks custom headers on PUT HTTP requests #834

dmunch opened this issue Mar 27, 2017 · 6 comments
Labels
Milestone

Comments

@dmunch
Copy link

dmunch commented Mar 27, 2017

Hi,

we are just about to upgrade to the latest 1.4 release and we're hitting a new issue concerning custom headers. We're in the same scenario as described in #687, however we now see request being send without the custom headers being set. We're still investigating for further details, but it seems like only requests for attachments are affected.

Best,
Daniel

@dmunch
Copy link
Author

dmunch commented Mar 27, 2017

Seems like fa8b121#diff-943160e99ba2b155e41572bfd0414d10 changed the way BulkDownloader makes HTTP request. If I judge the situation correctly no custom headers are appended.

The only occurence of BulkDownloader seems to be in

BulkDownloader dl = new BulkDownloader(new BulkDownloaderOptions {
, however Headers is not passed into BulkDownloaderOptions which might explain the fact that the missing headers only occur when downloading attachments.

@borrrden
Copy link
Member

The custom headers are part of the overall remote session object now, so they should persist throughout all parts of the replicator's lifetime. The reason they are not passed in anymore is because instead of creating a new http client for itself, it reuses the existing one.

That being said, the complexity of the replicator has probably led to another error on my part (this is one of the big overhauls being done for 2.0, to prevent these kinds of things from popping up again and again) so can you let me know how you are setting these headers?

@dmunch
Copy link
Author

dmunch commented Apr 3, 2017

Here's the snippet right from our code which sets the header, where replication is an instance of Couchbase.Lite.Replication

if (replication.Headers != null)
{
    replication.Headers = new Dictionary<string, string>();
}
replication.Headers.Add("Authorization", $"Bearer {bearerToken}");

@dmunch
Copy link
Author

dmunch commented Apr 3, 2017

Having a closer (and especially fresh look after the week-end) on the logs it's actually the uploading of attachments which fails. A small peek into the code might suggest that SendAsyncMultipartRequest doesn't set the required headers like the other Send* methods do.

i.e. a comparison of
https://github.com/couchbase/couchbase-lite-net/blob/master/src/Couchbase.Lite.Shared/Replication/RemoteSession.cs#L416

and

https://github.com/couchbase/couchbase-lite-net/blob/master/src/Couchbase.Lite.Shared/Replication/RemoteSession.cs#L292

might suggest that there's a AddRequestHeaders(message); missing.

@dmunch dmunch changed the title 1.4 breaks custom headers on certain HTTP requests 1.4 breaks custom headers on PUT HTTP requests Apr 3, 2017
@borrrden borrrden added bug and removed needsmoreinfo labels Apr 3, 2017
@borrrden borrrden added this to the 1.4.1 milestone Apr 3, 2017
@borrrden
Copy link
Member

borrrden commented Apr 3, 2017

That does seem quite likely. I'll put in that change and it will be in the next CI build soon (1.4.1-65 or higher)

borrrden added a commit that referenced this issue Apr 3, 2017
@borrrden borrrden added the ready label Apr 3, 2017
@dmunch
Copy link
Author

dmunch commented Apr 4, 2017

Perfect, 1.4.1-b0065 solves the issue. Thanks a lot for the quick reaction!

@dmunch dmunch closed this as completed Apr 4, 2017
@hideki hideki removed the ready label Apr 4, 2017
@djpongh djpongh modified the milestones: 1.4.2, 1.4.1 Apr 7, 2017
@borrrden borrrden modified the milestones: 1.4.1, 1.4.2 Sep 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants