Skip to content

Commit 6bfe61d

Browse files
author
oleksiys
committed
[1.9.x] + fix issue AsyncHttpClient#866
AsyncHttpClient#866 "NTLM auth fails on redirect"
1 parent c3d5d32 commit 6bfe61d

File tree

4 files changed

+98
-62
lines changed

4 files changed

+98
-62
lines changed

‎src/main/java/com/ning/http/client/providers/grizzly/AhcEventFilter.java

Lines changed: 89 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.ning.http.client.FluentCaseInsensitiveStringsMap;
2020
import com.ning.http.client.MaxRedirectException;
2121
import com.ning.http.client.Realm;
22+
import com.ning.http.client.Realm.AuthScheme;
2223
import com.ning.http.client.Request;
2324
import com.ning.http.client.RequestBuilder;
2425
import com.ning.http.client.cookie.CookieDecoder;
@@ -55,6 +56,7 @@
5556
import org.slf4j.LoggerFactory;
5657

5758
import static com.ning.http.client.providers.netty.util.HttpUtils.getNTLM;
59+
import static com.ning.http.util.AsyncHttpProviderUtils.*;
5860
import static com.ning.http.util.MiscUtils.isNonEmpty;
5961
/**
6062
* AHC {@link HttpClientFilter} implementation.
@@ -152,7 +154,8 @@ protected void onHttpContentEncoded(final HttpContent content,
152154
final AsyncHandler handler = context.getAsyncHandler();
153155
if (handler instanceof TransferCompletionHandler) {
154156
final int written = content.getContent().remaining();
155-
final long total = context.totalBodyWritten.addAndGet(written);
157+
context.totalBodyWritten += written;
158+
final long total = context.totalBodyWritten;
156159
((TransferCompletionHandler) handler).onContentWriteProgress(
157160
written, total, content.getHttpHeader().getContentLength());
158161
}
@@ -203,15 +206,13 @@ protected void onInitialLineParsed(final HttpHeader httpHeader,
203206
if (context.statusHandler == null) {
204207
context.statusHandler = RedirectHandler.INSTANCE;
205208
}
206-
context.redirectCount.incrementAndGet();
209+
context.redirectCount++;
207210
if (redirectCountExceeded(context)) {
208211
httpHeader.setSkipRemainder(true);
209212
context.abort(new MaxRedirectException());
210213
}
211214
} else {
212-
if (context.redirectCount.get() > 0) {
213-
context.redirectCount.set(0);
214-
}
215+
context.redirectCount = 0;
215216
}
216217
}
217218
final GrizzlyResponseStatus responseStatus =
@@ -460,7 +461,7 @@ private static void cleanup(final HttpContext httpContext) {
460461
}
461462

462463
private static boolean redirectCountExceeded(final HttpTransactionContext context) {
463-
return context.redirectCount.get() > context.maxRedirectCount;
464+
return context.redirectCount > context.maxRedirectCount;
464465
}
465466

466467
private static boolean isRedirect(final int status) {
@@ -497,10 +498,8 @@ public boolean handleStatus(final HttpResponsePacket responsePacket,
497498
final GrizzlyAsyncHttpProvider provider =
498499
httpTransactionContext.provider;
499500

500-
Realm realm = httpTransactionContext.getAhcRequest().getRealm();
501-
if (realm == null) {
502-
realm = provider.getClientConfig().getRealm();
503-
}
501+
Realm realm = getRealm(httpTransactionContext);
502+
504503
if (realm == null) {
505504
httpTransactionContext.invocationStatus = InvocationStatus.STOP;
506505
final AsyncHandler ah = httpTransactionContext.getAsyncHandler();
@@ -515,6 +514,7 @@ public boolean handleStatus(final HttpResponsePacket responsePacket,
515514
}
516515
return true;
517516
}
517+
518518
final Request req = httpTransactionContext.getAhcRequest();
519519

520520
try {
@@ -526,11 +526,14 @@ public boolean handleStatus(final HttpResponsePacket responsePacket,
526526
if (ntlmAuthenticate != null) {
527527
// NTLM
528528
// Connection-based auth
529-
isContinueAuth = true;
530-
531-
newRealm = ntlmChallenge(ctx.getConnection(),
529+
isContinueAuth = ntlmChallenge(ctx.getConnection(),
532530
ntlmAuthenticate, req,
533531
req.getHeaders(), realm);
532+
533+
newRealm = new Realm.RealmBuilder().clone(realm)//
534+
.setUri(req.getUri())//
535+
.setMethodName(req.getMethod())//
536+
.build();
534537
} else {
535538
// Request-based auth
536539
isContinueAuth = false;
@@ -539,7 +542,6 @@ public boolean handleStatus(final HttpResponsePacket responsePacket,
539542

540543
newRealm = new Realm.RealmBuilder()
541544
.clone(realm)
542-
.setScheme(realm.getScheme())
543545
.setUri(req.getUri())
544546
.setMethodName(req.getMethod())
545547
.setUsePreemptiveAuth(true)
@@ -564,7 +566,6 @@ public boolean handleStatus(final HttpResponsePacket responsePacket,
564566
}
565567

566568
final Request nextRequest = new RequestBuilder(req)
567-
.setHeaders(req.getHeaders())
568569
.setRealm(newRealm)
569570
.build();
570571

@@ -577,10 +578,8 @@ public boolean handleStatus(final HttpResponsePacket responsePacket,
577578

578579
try {
579580
provider.execute(newContext);
580-
return false;
581581
} catch (IOException ioe) {
582582
newContext.abort(ioe);
583-
return false;
584583
}
585584
} catch (Exception e) {
586585
httpTransactionContext.abort(e);
@@ -589,7 +588,7 @@ public boolean handleStatus(final HttpResponsePacket responsePacket,
589588
return false;
590589
}
591590

592-
private Realm ntlmChallenge(final Connection c,
591+
private boolean ntlmChallenge(final Connection c,
593592
final String wwwAuth, final Request request,
594593
final FluentCaseInsensitiveStringsMap headers,
595594
final Realm realm)
@@ -600,18 +599,16 @@ private Realm ntlmChallenge(final Connection c,
600599
String challengeHeader = NTLMEngine.INSTANCE.generateType1Msg();
601600

602601
addNTLMAuthorizationHeader(headers, challengeHeader, false);
602+
return true;
603603
} else {
604604
// probably receiving Type2Msg, so we issue Type3Msg
605605
addType3NTLMAuthorizationHeader(wwwAuth, headers, realm, false);
606606
// we mark NTLM as established for the Connection to
607607
// avoid preemptive NTLM
608608
Utils.setNtlmEstablished(c);
609+
610+
return false;
609611
}
610-
611-
return new Realm.RealmBuilder().clone(realm)//
612-
.setUri(request.getUri())//
613-
.setMethodName(request.getMethod())//
614-
.build();
615612
}
616613

617614
private void addNTLMAuthorizationHeader(
@@ -669,24 +666,18 @@ public boolean handleStatus(final HttpResponsePacket responsePacket,
669666
if (redirectURL == null) {
670667
throw new IllegalStateException("redirect received, but no location header was present");
671668
}
672-
673-
669+
674670
final Request req = httpTransactionContext.getAhcRequest();
675671
final GrizzlyAsyncHttpProvider provider = httpTransactionContext.provider;
676672

677-
final Uri orig = httpTransactionContext.lastRedirectURI == null
673+
final Uri origUri = httpTransactionContext.lastRedirectUri == null
678674
? req.getUri()
679-
: Uri.create(req.getUri(),
680-
httpTransactionContext.lastRedirectURI);
675+
: httpTransactionContext.lastRedirectUri;
681676

682-
httpTransactionContext.lastRedirectURI = redirectURL;
683-
Request requestToSend;
684-
Uri uri = Uri.create(orig, redirectURL);
685-
if (!uri.toUrl().equalsIgnoreCase(orig.toUrl())) {
686-
requestToSend = newRequest(uri, responsePacket,
687-
httpTransactionContext,
688-
sendAsGet(responsePacket, httpTransactionContext));
689-
} else {
677+
final Uri redirectUri = Uri.create(origUri, redirectURL);
678+
httpTransactionContext.lastRedirectUri = redirectUri;
679+
680+
if (redirectUri.equals(origUri)) {
690681
httpTransactionContext.statusHandler = null;
691682
httpTransactionContext.invocationStatus = InvocationStatus.CONTINUE;
692683
try {
@@ -695,28 +686,49 @@ public boolean handleStatus(final HttpResponsePacket responsePacket,
695686
} catch (Exception e) {
696687
httpTransactionContext.abort(e);
697688
}
689+
698690
return true;
699691
}
700-
final ConnectionManager m = provider.getConnectionManager();
692+
693+
final Request nextRequest = newRequest(httpTransactionContext,
694+
redirectUri, responsePacket,
695+
getRealm(httpTransactionContext),
696+
sendAsGet(responsePacket, httpTransactionContext));
697+
701698
try {
702-
final Connection c = m.openSync(requestToSend);
703-
if (switchingSchemes(orig, uri)) {
704-
try {
705-
notifySchemeSwitch(ctx, c, uri);
706-
} catch (IOException ioe) {
707-
httpTransactionContext.abort(ioe);
708-
}
699+
responsePacket.setSkipRemainder(true); // ignore the remainder of the response
700+
701+
final Connection c;
702+
703+
// @TODO we may want to ditch the keep-alive connection if the response payload is too large
704+
if (responsePacket.getProcessingState().isKeepAlive() &&
705+
isSameHostAndProtocol(origUri, redirectUri)) {
706+
// if it's HTTP keep-alive connection - reuse the
707+
// same Grizzly Connection
708+
c = ctx.getConnection();
709+
httpTransactionContext.reuseConnection();
710+
} else {
711+
// if it's not keep-alive - take new Connection from the pool
712+
final ConnectionManager m = provider.getConnectionManager();
713+
c = m.openSync(nextRequest);
709714
}
715+
710716
final HttpTransactionContext newContext =
711717
httpTransactionContext.cloneAndStartTransactionFor(
712-
c, requestToSend);
718+
c, nextRequest);
713719

714720
newContext.invocationStatus = InvocationStatus.CONTINUE;
715-
provider.execute(newContext);
721+
try {
722+
provider.execute(newContext);
723+
} catch (IOException ioe) {
724+
newContext.abort(ioe);
725+
}
726+
716727
return false;
717728
} catch (Exception e) {
718729
httpTransactionContext.abort(e);
719730
}
731+
720732
httpTransactionContext.invocationStatus = InvocationStatus.CONTINUE;
721733
return true;
722734
}
@@ -734,24 +746,48 @@ private boolean switchingSchemes(final Uri oldUri, final Uri newUri) {
734746
}
735747

736748
private void notifySchemeSwitch(final FilterChainContext ctx,
737-
final Connection c, final Uri uri) throws IOException {
749+
final Connection c, final Uri uri) {
738750
ctx.notifyDownstream(new SSLSwitchingEvent("https".equals(uri.getScheme()), c));
739751
}
740752
} // END RedirectHandler
753+
741754

742755
// ----------------------------------------------------- Private Methods
743-
private static Request newRequest(final Uri uri,
744-
final HttpResponsePacket response,
745-
final HttpTransactionContext ctx, boolean asGet) {
746-
final RequestBuilder builder = new RequestBuilder(ctx.getAhcRequest());
756+
private static Request newRequest(final HttpTransactionContext ctx,
757+
final Uri newUri, final HttpResponsePacket response,
758+
final Realm realm, boolean asGet) {
759+
final Request prototype = ctx.getAhcRequest();
760+
final FluentCaseInsensitiveStringsMap prototypeHeaders =
761+
prototype.getHeaders();
762+
763+
prototypeHeaders.remove(Header.Host.toString());
764+
prototypeHeaders.remove(Header.ContentLength.toString());
765+
766+
if (asGet)
767+
prototypeHeaders.remove(Header.ContentType.toString());
768+
if (realm != null && realm.getScheme() == AuthScheme.NTLM) {
769+
prototypeHeaders.remove(Header.Authorization.toString());
770+
prototypeHeaders.remove(Header.ProxyAuthorization.toString());
771+
}
772+
773+
final RequestBuilder builder = new RequestBuilder(prototype);
747774
if (asGet) {
748775
builder.setMethod("GET");
749776
}
750-
builder.setUrl(uri.toString());
751-
for (String cookieStr : response.getHeaders().values(Header.Cookie)) {
777+
builder.setUrl(newUri.toString());
778+
for (String cookieStr : response.getHeaders().values(Header.SetCookie)) {
752779
builder.addOrReplaceCookie(CookieDecoder.decode(cookieStr));
753780
}
781+
754782
return builder.build();
755783
}
756784

785+
private static Realm getRealm(final HttpTransactionContext httpTransactionContext) {
786+
final Realm realm = httpTransactionContext.getAhcRequest().getRealm();
787+
788+
return realm != null
789+
? realm
790+
: httpTransactionContext.provider.getClientConfig().getRealm();
791+
}
792+
757793
} // END AsyncHttpClientEventFilter

‎src/main/java/com/ning/http/client/providers/grizzly/HttpTransactionContext.java

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919
import com.ning.http.client.ws.WebSocket;
2020
import com.ning.http.util.AsyncHttpProviderUtils;
2121
import java.io.IOException;
22-
import java.util.concurrent.atomic.AtomicInteger;
23-
import java.util.concurrent.atomic.AtomicLong;
2422
import org.glassfish.grizzly.CloseListener;
2523
import org.glassfish.grizzly.CloseType;
2624
import org.glassfish.grizzly.Closeable;
@@ -43,7 +41,7 @@ public final class HttpTransactionContext {
4341
private static final Attribute<HttpTransactionContext> REQUEST_STATE_ATTR =
4442
Grizzly.DEFAULT_ATTRIBUTE_BUILDER.createAttribute(HttpTransactionContext.class.getName());
4543

46-
final AtomicInteger redirectCount = new AtomicInteger(0);
44+
int redirectCount;
4745
final int maxRedirectCount;
4846
final boolean redirectsAllowed;
4947
final GrizzlyAsyncHttpProvider provider;
@@ -65,8 +63,8 @@ public final class HttpTransactionContext {
6563
GrizzlyResponseStatus responseStatus;
6664

6765

68-
String lastRedirectURI;
69-
AtomicLong totalBodyWritten = new AtomicLong();
66+
Uri lastRedirectUri;
67+
long totalBodyWritten;
7068
AsyncHandler.STATE currentState;
7169
Uri wsRequestURI;
7270
boolean isWSRequest;
@@ -180,8 +178,8 @@ HttpTransactionContext cloneAndStartTransactionFor(
180178
newContext.payloadGenerator = payloadGenerator;
181179
newContext.currentState = currentState;
182180
newContext.statusHandler = statusHandler;
183-
newContext.lastRedirectURI = lastRedirectURI;
184-
newContext.redirectCount.set(redirectCount.get());
181+
newContext.lastRedirectUri = lastRedirectUri;
182+
newContext.redirectCount = redirectCount;
185183

186184
// detach the future
187185
future = null;

‎src/main/java/com/ning/http/client/providers/grizzly/PayloadGenFactory.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -443,7 +443,8 @@ public void updated(WriteResult result) {
443443
final AsyncHandler ah = context.getAsyncHandler();
444444
if (ah instanceof TransferCompletionHandler) {
445445
final long written = result.getWrittenSize();
446-
final long total = context.totalBodyWritten.addAndGet(written);
446+
context.totalBodyWritten += written;
447+
final long total = context.totalBodyWritten;
447448
((TransferCompletionHandler) ah).onContentWriteProgress(
448449
written,
449450
total,

‎src/main/java/com/ning/http/client/providers/grizzly/SwitchingSSLFilter.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@
2929
*/
3030
final class SwitchingSSLFilter extends SSLFilter {
3131
private final boolean secureByDefault;
32-
final Attribute<Boolean> CONNECTION_IS_SECURE = Grizzly.DEFAULT_ATTRIBUTE_BUILDER.createAttribute(SwitchingSSLFilter.class.getName());
32+
final Attribute<Boolean> CONNECTION_IS_SECURE =
33+
Grizzly.DEFAULT_ATTRIBUTE_BUILDER.createAttribute(SwitchingSSLFilter.class.getName());
3334
// -------------------------------------------------------- Constructors
3435

3536
SwitchingSSLFilter(final SSLEngineConfigurator clientConfig, final boolean secureByDefault) {

0 commit comments

Comments
 (0)