fix: allow delegate access request property (#11)

> nodejs.TypeError: Cannot read private member #cookies from an object whose class did not declare it  ## Summary by CodeRabbit - **New Features** - Introduced two new Koa application files (`helloworld.cjs` and `helloworld.mjs`) that respond with a greeting message including the client's IP address. - **Bug Fixes** - Enhanced cookie handling in the `Context` class. - Updated visibility of certain properties in the `Request` class for better accessibility. - **Tests** - Added a new test case to verify the correct retrieval of the client's IP address in the Koa application. - **Chores** - Updated dependency versions and added type definitions in `package.json`. - Modified import statements in various files for improved clarity and organization.
eggjs · Dec 11, 2024 · 2967724 · 2967724
1 parent b3c0d5c
commit 2967724
Show file tree

Hide file tree

Showing 7 changed files with 138 additions and 52 deletions.
diff --git a/example/helloworld.cjs b/example/helloworld.cjs
@@ -0,0 +1,11 @@
+// eslint-disable-next-line @typescript-eslint/no-var-requires
+const { Application } = require('../dist/commonjs/index.js');
+
+const app = new Application();
+
+// response
+app.use(ctx => {
+  ctx.body = `Hello Koa, from ${ctx.ip}`;
+});
+
+app.listen(3000);
diff --git a/example/helloworld.mjs b/example/helloworld.mjs
@@ -0,0 +1,10 @@
+import { Application } from '../dist/esm/index.js';
+
+const app = new Application();
+
+// response
+app.use(ctx => {
+  ctx.body = `Hello Koa, from ${ctx.ip}`;
+});
+
+app.listen(3000);
diff --git a/package.json b/package.json
@@ -30,18 +30,18 @@
   ],
   "license": "MIT",
   "dependencies": {
-    "accepts": "^1.3.5",
+    "accepts": "^1.3.8",
     "cache-content-type": "^2.0.0",
     "content-disposition": "~0.5.4",
     "content-type": "^1.0.5",
-    "cookies": "~0.8.0",
+    "cookies": "^0.9.1",
     "delegates": "^1.0.0",
     "destroy": "^1.0.4",
     "encodeurl": "^1.0.2",
     "escape-html": "^1.0.3",
     "fresh": "~0.5.2",
     "gals": "^1.0.1",
-    "http-assert": "^1.3.0",
+    "http-assert": "^1.5.0",
     "http-errors": "^2.0.0",
     "is-type-of": "^2.1.0",
     "koa-compose": "^4.1.0",
@@ -52,14 +52,18 @@
     "vary": "^1.1.2"
   },
   "devDependencies": {
-    "@arethetypeswrong/cli": "^0.15.3",
+    "@arethetypeswrong/cli": "^0.17.1",
     "@eggjs/tsconfig": "^1.3.3",
+    "@types/accepts": "^1.3.7",
+    "@types/content-disposition": "^0.5.8",
     "@types/content-type": "^1.1.8",
+    "@types/cookies": "^0.9.0",
     "@types/delegates": "^1.0.3",
     "@types/destroy": "^1.0.3",
     "@types/encodeurl": "^1.0.2",
     "@types/escape-html": "^1.0.4",
     "@types/fresh": "^0.5.2",
+    "@types/http-assert": "^1.5.6",
     "@types/http-errors": "^2.0.4",
     "@types/koa-compose": "^3.2.8",
     "@types/mocha": "^10.0.1",
@@ -76,7 +80,7 @@
     "mm": "^3.3.0",
     "supertest": "^3.1.0",
     "tsd": "^0.31.0",
-    "tshy": "^1.15.1",
+    "tshy": "3",
     "tshy-after": "^1.0.0",
     "typescript": "^5.4.5"
   },
@@ -91,12 +95,10 @@
     "./package.json": "./package.json",
     ".": {
       "import": {
-        "source": "./src/index.ts",
         "types": "./dist/esm/index.d.ts",
         "default": "./dist/esm/index.js"
       },
       "require": {
-        "source": "./src/index.ts",
         "types": "./dist/commonjs/index.d.ts",
         "default": "./dist/commonjs/index.js"
       }

diff --git a/src/context.ts b/src/context.ts
@@ -201,19 +201,20 @@ export class Context {
     res.end(msg);
   }
 
-  #cookies: Cookies;
+  protected _cookies: Cookies | undefined;
+
   get cookies() {
-    if (!this.#cookies) {
-      this.#cookies = new Cookies(this.req, this.res, {
+    if (!this._cookies) {
+      this._cookies = new Cookies(this.req, this.res, {
         keys: this.app.keys,
         secure: this.request.secure,
       });
     }
-    return this.#cookies;
+    return this._cookies;
   }
 
   set cookies(cookies: Cookies) {
-    this.#cookies = cookies;
+    this._cookies = cookies;
   }
 }
 

diff --git a/src/request.ts b/src/request.ts
@@ -1,11 +1,9 @@
-import net from 'node:net';
-import type { Socket } from 'node:net';
+import net, { type Socket } from 'node:net';
 import { format as stringify } from 'node:url';
-import qs from 'node:querystring';
+import qs, { type ParsedUrlQuery } from 'node:querystring';
 import util from 'node:util';
-import type { ParsedUrlQuery } from 'node:querystring';
 import type { IncomingMessage, ServerResponse } from 'node:http';
-import accepts from 'accepts';
+import accepts, { type Accepts } from 'accepts';
 import contentType from 'content-type';
 import parse from 'parseurl';
 import typeis from 'type-is';
@@ -134,19 +132,19 @@ export class Request {
     this.url = stringify(url);
   }
 
-  #parsedUrlQueryCache: Record<string, ParsedUrlQuery>;
+  protected _parsedUrlQueryCache: Record<string, ParsedUrlQuery> | undefined;
 
   /**
    * Get parsed query string.
    */
   get query() {
     const str = this.querystring;
-    if (!this.#parsedUrlQueryCache) {
-      this.#parsedUrlQueryCache = {};
+    if (!this._parsedUrlQueryCache) {
+      this._parsedUrlQueryCache = {};
     }
-    let parsedUrlQuery = this.#parsedUrlQueryCache[str];
+    let parsedUrlQuery = this._parsedUrlQueryCache[str];
     if (!parsedUrlQuery) {
-      parsedUrlQuery = this.#parsedUrlQueryCache[str] = qs.parse(str);
+      parsedUrlQuery = this._parsedUrlQueryCache[str] = qs.parse(str);
     }
     return parsedUrlQuery;
   }
@@ -210,10 +208,16 @@ export class Request {
     const proxy = this.app.proxy;
     let host = proxy ? this.get<string>('X-Forwarded-Host') : '';
     if (!host) {
-      if (this.req.httpVersionMajor >= 2) host = this.get(':authority');
-      if (!host) host = this.get('Host');
+      if (this.req.httpVersionMajor >= 2) {
+        host = this.get(':authority');
+      }
+      if (!host) {
+        host = this.get('Host');
+      }
+    }
+    if (!host) {
+      return '';
     }
-    if (!host) return '';
     return host.split(/\s*,\s*/, 1)[0];
   }
 
@@ -224,27 +228,31 @@ export class Request {
    */
   get hostname() {
     const host = this.host;
-    if (!host) return '';
-    if (host[0] === '[') return this.URL.hostname || ''; // IPv6
+    if (!host) {
+      return '';
+    }
+    if (host[0] === '[') {
+      return this.URL.hostname || ''; // IPv6
+    }
     return host.split(':', 1)[0];
   }
 
-  #memoizedURL: URL;
+  protected _memoizedURL: URL | undefined;
 
   /**
    * Get WHATWG parsed URL.
    * Lazily memoized.
    */
   get URL() {
-    if (!this.#memoizedURL) {
+    if (!this._memoizedURL) {
       const originalUrl = this.originalUrl || ''; // avoid undefined in template string
       try {
-        this.#memoizedURL = new URL(`${this.origin}${originalUrl}`);
+        this._memoizedURL = new URL(`${this.origin}${originalUrl}`);
       } catch {
-        this.#memoizedURL = Object.create(null);
+        this._memoizedURL = Object.create(null);
       }
     }
-    return this.#memoizedURL;
+    return this._memoizedURL!;
   }
 
   /**
@@ -257,7 +265,9 @@ export class Request {
     const status = this.response.status;
 
     // GET or HEAD for weak freshness validation only
-    if (method !== 'GET' && method !== 'HEAD') return false;
+    if (method !== 'GET' && method !== 'HEAD') {
+      return false;
+    }
 
     // 2xx or 304 as per rfc2616 14.26
     if ((status >= 200 && status < 300) || status === 304) {
@@ -308,7 +318,9 @@ export class Request {
    */
   get length() {
     const len = this.get<string>('Content-Length');
-    if (len === '') return;
+    if (len === '') {
+      return;
+    }
     return parseInt(len);
   }
 
@@ -321,8 +333,12 @@ export class Request {
    * may be enabled.
    */
   get protocol() {
-    if (this.socket.encrypted) return 'https';
-    if (!this.app.proxy) return 'http';
+    if (this.socket.encrypted) {
+      return 'https';
+    }
+    if (!this.app.proxy) {
+      return 'http';
+    }
     const proto = this.get<string>('X-Forwarded-Proto');
     return proto ? proto.split(/\s*,\s*/, 1)[0] : 'http';
   }
@@ -356,21 +372,21 @@ export class Request {
     return ips;
   }
 
-  #ip: string;
+  protected _ip: string;
   /**
    * Return request's remote address
    * When `app.proxy` is `true`, parse
    * the "X-Forwarded-For" ip address list and return the first one
    */
   get ip() {
-    if (!this.#ip) {
-      this.#ip = this.ips[0] || this.socket.remoteAddress || '';
+    if (!this._ip) {
+      this._ip = this.ips[0] || this.socket.remoteAddress || '';
     }
-    return this.#ip;
+    return this._ip;
   }
 
   set ip(ip: string) {
-    this.#ip = ip;
+    this._ip = ip;
   }
 
   /**
@@ -395,20 +411,20 @@ export class Request {
       .slice(offset);
   }
 
-  #accept: any;
+  protected _accept: Accepts;
   /**
    * Get accept object.
    * Lazily memoized.
    */
   get accept() {
-    return this.#accept || (this.#accept = accepts(this.req));
+    return this._accept || (this._accept = accepts(this.req));
   }
 
   /**
    * Set accept object.
    */
-  set accept(obj) {
-    this.#accept = obj;
+  set accept(obj: Accepts) {
+    this._accept = obj;
   }
 
   /**
@@ -459,8 +475,19 @@ export class Request {
    *
    *     ['gzip', 'deflate']
    */
-  acceptsEncodings(...args: any[]): string | string[] {
-    return this.accept.encodings(...args);
+  acceptsEncodings(): string[];
+  acceptsEncodings(encodings: string[]): string | false;
+  acceptsEncodings(...encodings: string[]): string | false;
+  acceptsEncodings(encodings?: string | string[], ...others: string[]): string[] | string | false {
+    if (!encodings) {
+      return this.accept.encodings();
+    }
+    if (Array.isArray(encodings)) {
+      encodings = [ ...encodings, ...others ];
+    } else {
+      encodings = [ encodings, ...others ];
+    }
+    return this.accept.encodings(...encodings);
   }
 
   /**
@@ -471,8 +498,19 @@ export class Request {
    *
    *     ['utf-8', 'utf-7', 'iso-8859-1']
    */
-  acceptsCharsets(...args: any[]): string | string[] {
-    return this.accept.charsets(...args);
+  acceptsCharsets(): string[];
+  acceptsCharsets(charsets: string[]): string | false;
+  acceptsCharsets(...charsets: string[]): string | false;
+  acceptsCharsets(charsets?: string | string[], ...others: string[]): string[] | string | false {
+    if (!charsets) {
+      return this.accept.charsets();
+    }
+    if (Array.isArray(charsets)) {
+      charsets = [ ...charsets, ...others ];
+    } else {
+      charsets = [ charsets, ...others ];
+    }
+    return this.accept.charsets(...charsets);
   }
 
   /**
@@ -483,8 +521,19 @@ export class Request {
    *
    *     ['es', 'pt', 'en']
    */
-  acceptsLanguages(...args: any[]): string | string[] {
-    return this.accept.languages(...args);
+  acceptsLanguages(): string[];
+  acceptsLanguages(languages: string[]): string | false;
+  acceptsLanguages(...languages: string[]): string | false;
+  acceptsLanguages(languages?: string | string[], ...others: string[]): string | string[] | false {
+    if (!languages) {
+      return this.accept.languages();
+    }
+    if (Array.isArray(languages)) {
+      languages = [ ...languages, ...others ];
+    } else {
+      languages = [ languages, ...others ];
+    }
+    return this.accept.languages(...languages);
   }
 
   /**

diff --git a/src/response.ts b/src/response.ts
@@ -4,7 +4,7 @@ import util from 'node:util';
 import Stream from 'node:stream';
 import type { IncomingMessage, ServerResponse } from 'node:http';
 import contentDisposition from 'content-disposition';
-import getType from 'cache-content-type';
+import { getType } from 'cache-content-type';
 import onFinish from 'on-finished';
 import escape from 'escape-html';
 import { is as typeis } from 'type-is';

diff --git a/test/application/request.test.ts b/test/application/request.test.ts
@@ -28,4 +28,17 @@ describe('app.request', () => {
       .get('/')
       .expect(204);
   });
+
+  it('should access ip work', () => {
+    const app = new Koa();
+    app.use(ctx => {
+      ctx.status = 200;
+      ctx.body = ctx.request.ip;
+    });
+
+    return request(app.listen())
+      .get('/')
+      .expect(200)
+      .expect('::ffff:127.0.0.1');
+  });
 });