diff --git a/lib/context.ts b/lib/context.ts index 24bf723..1412b4a 100644 --- a/lib/context.ts +++ b/lib/context.ts @@ -19,11 +19,13 @@ 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"; @@ -31,6 +33,7 @@ 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 @@ -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 ) @@ -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 > { @@ -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 ) => @@ -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 ) => @@ -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 ) { diff --git a/lib/core.ts b/lib/core.ts index e579346..93834bf 100644 --- a/lib/core.ts +++ b/lib/core.ts @@ -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"; @@ -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; -} diff --git a/lib/fetch-common.ts b/lib/fetch-common.ts index 1c5a333..03662f8 100644 --- a/lib/fetch-common.ts +++ b/lib/fetch-common.ts @@ -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"; diff --git a/lib/fetch-http1.ts b/lib/fetch-http1.ts index 0b1bea0..728c7ab 100644 --- a/lib/fetch-http1.ts +++ b/lib/fetch-http1.ts @@ -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, @@ -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 @@ -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' @@ -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 ), } - ) - ); + ) ); + } } ) ); } ); @@ -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 ); } diff --git a/lib/fetch-http2.ts b/lib/fetch-http2.ts index 1ea06db..fee409a 100644 --- a/lib/fetch-http2.ts +++ b/lib/fetch-http2.ts @@ -11,8 +11,8 @@ import { AbortError, RetryError, FetchInit, - SimpleSessionHttp2, } from "./core"; +import { SimpleSessionHttp2 } from "./simple-session"; import { FetchExtra, handleSignalAndTimeout, @@ -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 { @@ -121,6 +127,7 @@ async function fetchImpl( }; let stream: ClientHttp2Stream; + let shouldCleanupSocket = true; try { stream = h2session.request( headersToSend, { endStream } ); @@ -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. @@ -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' @@ -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 ), + } + ) ); + } } ) ); } ); @@ -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 ); } diff --git a/lib/simple-session.ts b/lib/simple-session.ts new file mode 100644 index 0000000..be2db18 --- /dev/null +++ b/lib/simple-session.ts @@ -0,0 +1,50 @@ +import { ClientRequest } from "http"; +import { ClientHttp2Session } from "http2"; + +import { CookieJar } from "./cookie-jar"; +import { HttpProtocols, Decoder, FetchInit } from "./core"; +import { FetchExtra } from "./fetch-common"; +import { Request } from "./request"; +import { Response } from "./response"; + + +export interface SimpleSession +{ + protocol: HttpProtocols; + + cookieJar: CookieJar; + + userAgent( ): string; + accept( ): string; + + contentDecoders( ): ReadonlyArray< Decoder >; + + newFetch( + input: string | Request, + init?: Partial< FetchInit >, + extra?: FetchExtra + ) + : Promise< Response >; +} + +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; +} diff --git a/lib/utils.ts b/lib/utils.ts index 21e3171..4c7eaad 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -22,15 +22,27 @@ export function arrayify< T >( : [ value ]; } +export interface ParsedLocation +{ + url: string; + isRelative: boolean; +} + export function parseLocation( location: string | Array< string > | undefined, origin: string ) +: null | ParsedLocation { if ( "string" !== typeof location ) return null; + const originUrl = new URL( origin ); const url = new URL( location, origin ); - return url.href; + + return { + url: url.href, + isRelative: originUrl.origin === url.origin, + }; } export const isRedirectStatus: { [ status: string ]: boolean; } = { diff --git a/test-client/index.ts b/test-client/index.ts index c09ea14..0f3eff4 100755 --- a/test-client/index.ts +++ b/test-client/index.ts @@ -29,6 +29,7 @@ async function work( ) url, { method: < any >method, + redirect: 'follow', } ); diff --git a/test/fetch-h2/event-loop-reference.ts b/test/fetch-h2/event-loop-reference.ts index ac29f7e..5e4b9e1 100644 --- a/test/fetch-h2/event-loop-reference.ts +++ b/test/fetch-h2/event-loop-reference.ts @@ -6,7 +6,8 @@ import { TestData } from "../lib/server-common"; import { makeMakeServer } from "../lib/server-helpers"; -const script = path.resolve( path.join( process.cwd( ), "scripts", "test-client" ) ); +const script = + path.resolve( path.join( process.cwd( ), "scripts", "test-client" ) ); describe( "event-loop", ( ) => { @@ -42,5 +43,44 @@ describe( "event-loop", ( ) => await server.shutdown( ); } ); + + it( `should handle redirect ${proto} ${version}`, async ( ) => + { + const { port, server } = await makeServer( ); + + const url = `${proto}//localhost:${port}/redirect/delay/50`; + + const body = { foo: "bar" }; + + const { stdout } = await execa( + script, + [ "GET", url, version, "insecure" ], + { input: JSON.stringify( body ), stderr: 'inherit' } + ); + + expect( stdout ).toBe( "abcdefghij" ); + + await server.shutdown( ); + } ); + + it( `should handle absolute redirect ${proto} ${version}`, async ( ) => + { + const { port, server } = await makeServer( ); + + const redirectTo = `${proto}//localhost:${port}/delay/50`; + const url = `${proto}//localhost:${port}/redirect/${redirectTo}`; + + const body = { foo: "bar" }; + + const { stdout } = await execa( + script, + [ "GET", url, version, "insecure" ], + { input: JSON.stringify( body ), stderr: 'inherit' } + ); + + expect( stdout ).toBe( "abcdefghij" ); + + await server.shutdown( ); + } ); } ); } ); diff --git a/test/lib/server-http1.ts b/test/lib/server-http1.ts index 2720944..4a3ed95 100644 --- a/test/lib/server-http1.ts +++ b/test/lib/server-http1.ts @@ -33,6 +33,7 @@ const { HTTP2_HEADER_CONTENT_LENGTH, HTTP2_HEADER_CONTENT_TYPE, HTTP2_HEADER_SET_COOKIE, + HTTP2_HEADER_LOCATION, } = h2constants; interface RawHeaders @@ -286,6 +287,21 @@ export class ServerHttp1 extends TypedServer< HttpServer | HttpsServer > { request.socket.destroy( ); } + else if ( path.startsWith( "/redirect/" ) ) + { + const redirectTo = + path.slice( 10 ).startsWith( "http" ) + ? path.slice( 10 ) + : path.slice( 9 ); + + const responseHeaders = { + ":status": 302, + [ HTTP2_HEADER_LOCATION ]: redirectTo, + }; + + sendHeaders( responseHeaders ); + response.end( ); + } else { response.end( ); diff --git a/test/lib/server-http2.ts b/test/lib/server-http2.ts index 36fe2fe..aaa580a 100644 --- a/test/lib/server-http2.ts +++ b/test/lib/server-http2.ts @@ -29,6 +29,7 @@ const { HTTP2_HEADER_CONTENT_LENGTH, HTTP2_HEADER_ACCEPT_ENCODING, HTTP2_HEADER_SET_COOKIE, + HTTP2_HEADER_LOCATION, } = constants; export class ServerHttp2 extends TypedServer< Http2Server > @@ -326,6 +327,21 @@ export class ServerHttp2 extends TypedServer< Http2Server > { stream.close( ); } + else if ( path.startsWith( "/redirect/" ) ) + { + const redirectTo = + path.slice( 10 ).startsWith( "http" ) + ? path.slice( 10 ) + : path.slice( 9 ); + + const responseHeaders = { + ":status": 302, + [ HTTP2_HEADER_LOCATION ]: redirectTo, + }; + + stream.respond( responseHeaders ); + stream.end( ); + } else { const matched = ( this._opts.matchers || [ ] )