Skip to content

Commit

Permalink
feat(core): added support for absolute redirections
Browse files Browse the repository at this point in the history
Also fixed async ref issue with http2 relative redirections

fix #107
  • Loading branch information
grantila committed Aug 20, 2020
1 parent fbc90bd commit c22c63c
Show file tree
Hide file tree
Showing 11 changed files with 243 additions and 96 deletions.
26 changes: 17 additions & 9 deletions lib/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,21 @@ import {
HttpProtocols,
parsePerOrigin,
PerOrigin,
RetryError,
} from "./core";
import {
SimpleSession,
SimpleSessionHttp1,
SimpleSessionHttp2,
RetryError,
} from "./core";
} from "./simple-session";
import { fetch as fetchHttp1 } from "./fetch-http1";
import { fetch as fetchHttp2 } from "./fetch-http2";
import { version } from "./generated/version";
import { Request } from "./request";
import { Response } from "./response";
import { parseInput } from "./utils";
import OriginCache from "./origin-cache";
import { FetchExtra } from "./fetch-common";


function makeDefaultUserAgent( ): string
Expand Down Expand Up @@ -174,7 +177,7 @@ export class Context

public async fetch( input: string | Request, init?: Partial< FetchInit > )
{
return this.retryFetch( input, init, 0 );
return this.retryFetch( input, init );
}

public async disconnect( url: string )
Expand All @@ -201,25 +204,27 @@ export class Context
private async retryFetch(
input: string | Request,
init: Partial< FetchInit > | undefined,
count: number
extra?: FetchExtra,
count: number = 0
)
: Promise< Response >
{
++count;

return this.retryableFetch( input, init )
return this.retryableFetch( input, init, extra )
.catch( specific( RetryError, err =>
{
// TODO: Implement a more robust retry logic
if ( count > 10 )
throw err;
return this.retryFetch( input, init, count );
return this.retryFetch( input, init, extra, count );
} ) );
}

private async retryableFetch(
input: string | Request,
init?: Partial< FetchInit >
init?: Partial< FetchInit >,
extra?: FetchExtra
)
: Promise< Response >
{
Expand All @@ -243,6 +248,7 @@ export class Context
cookieJar: this._cookieJar,
protocol,
userAgent: ( ) => this.userAgent( origin ),
newFetch: this.retryFetch.bind( this ),
} );

const doFetchHttp1 = ( socket: Socket, cleanup: ( ) => void ) =>
Expand All @@ -259,7 +265,7 @@ export class Context
} ),
...makeSimpleSession( "http1" ),
};
return fetchHttp1( sessionGetterHttp1, request, init );
return fetchHttp1( sessionGetterHttp1, request, init, extra );
};

const doFetchHttp2 = async ( cacheableSession: CacheableH2Session ) =>
Expand All @@ -273,7 +279,9 @@ export class Context
get: ( ) => ( { session, cleanup } ),
...makeSimpleSession( "http2" ),
};
return await fetchHttp2( sessionGetterHttp2, request, init );
return await fetchHttp2(
sessionGetterHttp2, request, init, extra
);
}
catch ( err )
{
Expand Down
38 changes: 0 additions & 38 deletions lib/core.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
import { ClientRequest } from "http";
import { ClientHttp2Session } from "http2";

import { AbortSignal } from "./abort";
import { CookieJar } from "./cookie-jar";
import { Headers, RawHeaders } from "./headers";


Expand Down Expand Up @@ -241,37 +237,3 @@ export interface Http1Options
maxFreeSockets: number | PerOrigin< number >;
timeout: void | number | PerOrigin< void | number >;
}

export interface SimpleSession
{
protocol: HttpProtocols;

cookieJar: CookieJar;

userAgent( ): string;
accept( ): string;

contentDecoders( ): ReadonlyArray< Decoder >;
}

export interface SimpleSessionHttp1Request
{
req: ClientRequest;
cleanup: ( ) => void;
}

export interface SimpleSessionHttp2Session
{
session: Promise< ClientHttp2Session >;
cleanup: ( ) => void;
}

export interface SimpleSessionHttp1 extends SimpleSession
{
get( url: string ): SimpleSessionHttp1Request;
}

export interface SimpleSessionHttp2 extends SimpleSession
{
get( ): SimpleSessionHttp2Session;
}
9 changes: 2 additions & 7 deletions lib/fetch-common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,8 @@ import { URL } from "url";
import { Finally, rethrow } from "already";

import { BodyInspector } from "./body";
import {
AbortError,
Decoder,
FetchInit,
SimpleSession,
TimeoutError,
} from "./core";
import { AbortError, Decoder, FetchInit, TimeoutError } from "./core";
import { SimpleSession } from "./simple-session";
import { Headers, RawHeaders } from "./headers";
import { Request } from "./request";
import { Response } from "./response";
Expand Down
65 changes: 43 additions & 22 deletions lib/fetch-http1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,8 @@ import { Socket } from "net";
import { syncGuard } from "callguard";

import { AbortController } from "./abort";
import {
FetchInit,
SimpleSessionHttp1,
} from "./core";
import { FetchInit } from "./core";
import { SimpleSessionHttp1 } from "./simple-session";
import {
FetchExtra,
handleSignalAndTimeout,
Expand All @@ -23,7 +21,13 @@ import {
import { GuardedHeaders } from "./headers";
import { Request } from "./request";
import { Response, StreamResponse } from "./response";
import { arrayify, isRedirectStatus, parseLocation, pipeline } from "./utils";
import {
arrayify,
isRedirectStatus,
parseLocation,
pipeline,
ParsedLocation,
} from "./utils";

const {
// Responses, these are the same in HTTP/1.1 and HTTP/2
Expand Down Expand Up @@ -215,8 +219,11 @@ export async function fetchImpl(
)
);

const { url: locationUrl, isRelative } =
location as ParsedLocation;

if ( redirect === "error" )
return reject( makeRedirectionError( location ) );
return reject( makeRedirectionError( locationUrl ) );

// redirect is 'follow'

Expand All @@ -225,24 +232,36 @@ export async function fetchImpl(
// body). The concept is fundementally broken anyway...
if ( !endStream )
return reject(
makeRedirectionMethodError( location, method )
makeRedirectionMethodError( locationUrl, method )
);

if ( !location )
return reject( makeIllegalRedirectError( ) );

res.destroy( );
resolve(
fetchImpl(
session,
request.clone( location ),
{ signal, onTrailers },

if ( isRelative )
{
resolve(
fetchImpl(
session,
request.clone( locationUrl ),
{ signal, onTrailers },
{
redirected: redirected.concat( url ),
timeoutAt,
}
)
);
}
else
{
resolve( session.newFetch(
request.clone( locationUrl ),
init,
{
redirected: redirected.concat( url ),
timeoutAt,
redirected: redirected.concat( url ),
}
)
);
) );
}
} ) );
} );

Expand Down Expand Up @@ -274,13 +293,15 @@ export async function fetchImpl(
export function fetch(
session: SimpleSessionHttp1,
input: Request,
init?: Partial< FetchInit >
init?: Partial< FetchInit >,
extra?: FetchExtra
)
: Promise< Response >
{
const timeoutAt = void 0;

const extra = { timeoutAt, redirected: [ ] };
extra = {
timeoutAt: extra?.timeoutAt,
redirected: extra?.redirected ?? [ ],
};

return fetchImpl( session, input, init, extra );
}
62 changes: 44 additions & 18 deletions lib/fetch-http2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import {
AbortError,
RetryError,
FetchInit,
SimpleSessionHttp2,
} from "./core";
import { SimpleSessionHttp2 } from "./simple-session";
import {
FetchExtra,
handleSignalAndTimeout,
Expand All @@ -27,7 +27,13 @@ import {
import { GuardedHeaders } from "./headers";
import { Request } from "./request";
import { Response, StreamResponse } from "./response";
import { arrayify, isRedirectStatus, parseLocation, pipeline } from "./utils";
import {
arrayify,
isRedirectStatus,
parseLocation,
pipeline,
ParsedLocation,
} from "./utils";
import { hasGotGoaway } from "./utils-http2";

const {
Expand Down Expand Up @@ -121,6 +127,7 @@ async function fetchImpl(
};

let stream: ClientHttp2Stream;
let shouldCleanupSocket = true;
try
{
stream = h2session.request( headersToSend, { endStream } );
Expand Down Expand Up @@ -177,7 +184,8 @@ async function fetchImpl(

stream.on( "close", guard( ( ) =>
{
socketCleanup( );
if ( shouldCleanupSocket )
socketCleanup( );

// We'll get an 'error' event if there actually is an
// error, but not if we got NGHTTP2_NO_ERROR.
Expand Down Expand Up @@ -313,8 +321,11 @@ async function fetchImpl(
)
);

const { url: locationUrl, isRelative } =
location as ParsedLocation;

if ( redirect === "error" )
return reject( makeRedirectionError( location ) );
return reject( makeRedirectionError( locationUrl ) );

// redirect is 'follow'

Expand All @@ -323,25 +334,38 @@ async function fetchImpl(
// body). The concept is fundementally broken anyway...
if ( !endStream )
return reject(
makeRedirectionMethodError( location, method )
makeRedirectionMethodError( locationUrl, method )
);

if ( !location )
return reject( makeIllegalRedirectError( ) );

stream.destroy( );
resolve(
fetchImpl(
if ( isRelative )
{
shouldCleanupSocket = false;
stream.destroy( );
resolve( fetchImpl(
session,
request.clone( location ),
{ signal, onTrailers },
request.clone( locationUrl ),
init,
{
raceConditionedGoaway,
redirected: redirected.concat( url ),
timeoutAt,
}
)
);
) );
}
else
{
resolve( session.newFetch(
request.clone( locationUrl ),
init,
{
timeoutAt,
redirected: redirected.concat( url ),
}
) );
}
} ) );
} );

Expand Down Expand Up @@ -371,14 +395,16 @@ async function fetchImpl(
export function fetch(
session: SimpleSessionHttp2,
input: Request,
init?: Partial< FetchInit >
init?: Partial< FetchInit >,
extra?: FetchExtra
)
: Promise< Response >
{
const timeoutAt = void 0;

const raceConditionedGoaway = new Set< string>( );
const extra = { timeoutAt, redirected: [ ], raceConditionedGoaway };
const http2Extra: FetchExtraHttp2 = {
timeoutAt: extra?.timeoutAt,
redirected: extra?.redirected ?? [ ],
raceConditionedGoaway: new Set< string>( ),
};

return fetchImpl( session, input, init, extra );
return fetchImpl( session, input, init, http2Extra );
}
Loading

0 comments on commit c22c63c

Please sign in to comment.