WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

fix: Keep lightweight request even if accessing method, url or headers #274

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.

Sign up for GitHub

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

Merged
yusukebe merged 1 commit into from
Aug 30, 2025
Merged

fix: Keep lightweight request even if accessing method, url or headers #274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 19 additions & 10 deletions src/request.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,7 @@ export class Request extends GlobalRequest {
}
}

export type IncomingMessageWithWrapBodyStream = IncomingMessage & { [wrapBodyStream]: boolean }
export const wrapBodyStream = Symbol('wrapBodyStream')
const newRequestFromIncoming = (
method: string,
url: string,
incoming: IncomingMessage | Http2ServerRequest,
abortController: AbortController
): Request => {
const newHeadersFromIncoming = (incoming: IncomingMessage | Http2ServerRequest) => {
const headerRecord: [string, string][] = []
const rawHeaders = incoming.rawHeaders
for (let i = 0; i < rawHeaders.length; i += 2) {
Expand All @@ -58,10 +51,21 @@ const newRequestFromIncoming = (
headerRecord.push([key, value])
}
}
return new Headers(headerRecord)
Copy link
Member

@yusukebe yusukebe Aug 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using new Headers instead of creating Request is nice.

It's just an idea, maybe overwork, we should not add it in this PR, but is creating LightweightHeaders a good idea?

class LightweightHeaders {
  constructor(rawHeaders) {} // or constructor(headerRecord)

  get(name) {
    // find the value from rawHeaders or headerRecord
  }

  has(name) {
    // find the value from rawHeaders or headerRecord
  }

  set(name, value) {
    this.toHeaders.set(name, value)
  }

  toHeaders() {
    const headers = new Headers(this.entries())
    // cache headers
    return headers
  }
}

It's for readonly, so it returns the value by .get() from rawHeaders or headerRecord, and if accessing .set(), do new Headers().

Anyway, we can merge this PR.

Copy link
Member Author

@usualoma usualoma Aug 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LightweightHeaders is a great idea! However, I'm concerned that using it like fetch(url, {headers: c.req.raw.headers}) would introduce more compatibility issues to worry about, making it not worth the maintenance cost.

}

export type IncomingMessageWithWrapBodyStream = IncomingMessage & { [wrapBodyStream]: boolean }
export const wrapBodyStream = Symbol('wrapBodyStream')
const newRequestFromIncoming = (
method: string,
url: string,
headers: Headers,
incoming: IncomingMessage | Http2ServerRequest,
abortController: AbortController
): Request => {
const init = {
method: method,
headers: headerRecord,
headers,
signal: abortController.signal,
} as RequestInit

Expand Down Expand Up @@ -116,6 +120,7 @@ const getRequestCache = Symbol('getRequestCache')
const requestCache = Symbol('requestCache')
const incomingKey = Symbol('incomingKey')
const urlKey = Symbol('urlKey')
const headersKey = Symbol('headersKey')
export const abortControllerKey = Symbol('abortControllerKey')
export const getAbortController = Symbol('getAbortController')

Expand All @@ -128,6 +133,10 @@ const requestPrototype: Record<string | symbol, any> = {
return this[urlKey]
},

get headers() {
return (this[headersKey] ||= newHeadersFromIncoming(this[incomingKey]))
},

[getAbortController]() {
this[getRequestCache]()
return this[abortControllerKey]
Expand All @@ -138,6 +147,7 @@ const requestPrototype: Record<string | symbol, any> = {
return (this[requestCache] ||= newRequestFromIncoming(
this.method,
this[urlKey],
this.headers,
this[incomingKey],
this[abortControllerKey]
))
Expand All @@ -149,7 +159,6 @@ const requestPrototype: Record<string | symbol, any> = {
'cache',
'credentials',
'destination',
'headers',
'integrity',
'mode',
'redirect',
Expand Down
21 changes: 21 additions & 0 deletions test/request.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,27 @@ describe('Request', () => {
expect(req.keepalive).toBe(false)
})

it('Should not generate GlobalRequest when accessing method, url or headers', async () => {
const req = newRequest({
method: 'GET',
url: '/',
headers: {
host: 'localhost',
},
rawHeaders: ['host', 'localhost'],
} as IncomingMessage)

// keep lightweight request even if accessing method, url or headers
expect(req.method).toBe('GET')
expect(req.url).toBe('http://localhost/')
expect(req.headers.get('host')).toBe('localhost')
expect(req[abortControllerKey]).toBeUndefined()

// generate GlobalRequest
expect(req.keepalive).toBe(false)
expect(req[abortControllerKey]).toBeDefined()
})

it('Should resolve double dots in URL', async () => {
const req = newRequest({
headers: {
Expand Down
Loading