also looking at this
fix(browser)!: refuse orWhere rather than sending it as an AND
#1131
2 files
+136
-34
| @@ -401,23 +401,19 @@ export class BrowserQueryBuilder<T = any> { | ||
| 401 | 401 | */ |
| 402 | 402 | orWhere(column: string, value: unknown): this |
| 403 | 403 | orWhere(column: string, operator: WhereOperator, value: unknown): this |
| 404 | orWhere(column: string, operatorOrValue: unknown, value?: unknown): this { | |
| 405 | if (value === undefined) { | |
| 406 | this.state.wheres.push({ | |
| 407 | column, | |
| 408 | operator: '=', | |
| 409 | value: operatorOrValue, | |
| 410 | boolean: 'or', | |
| 411 | }) | |
| 412 | } | |
| 413 | else { | |
| 414 | this.state.wheres.push({ | |
| 415 | column, | |
| 416 | operator: operatorOrValue as WhereOperator, | |
| 417 | value, | |
| 418 | boolean: 'or', | |
| 419 | }) | |
| 420 | } | |
| 404 | orWhere(_column: string, _operatorOrValue: unknown, _value?: unknown): this { | |
| 405 | // Refused rather than recorded. `boolean: 'or'` was pushed onto the where | |
| 406 | // list and buildQueryParams never read it, so the term went out as a plain | |
| 407 | // filter and any receiving API read the whole set as a conjunction: | |
| 408 | // | |
| 409 | // .where('role', 'admin').orWhere('role', 'moderator') | |
| 410 | // -> role=admin&role=moderator (AND, not OR) | |
| 411 | // | |
| 412 | // That is a superset of the intended rows, returned without complaint. A | |
| 413 | // wire spelling for OR would have to be agreed with whatever serves these | |
| 414 | // requests, and inventing one here would ship a format nothing reads — | |
| 415 | // the same silent failure, one layer further away. See #1096. | |
| 416 | throw new TypeError('[browser] orWhere() is not supported by this client. The query-param format it builds has no spelling for a disjunction, so the term would be sent as a plain filter and the server would read it as AND \u2014 silently returning a superset of the rows you asked for. Express the alternatives as a single filter (whereIn), or issue one request per branch and merge the results.') | |
| 421 | 417 | return this |
| 422 | 418 | } |
| 423 | 419 | |
| @@ -536,8 +532,8 @@ export class BrowserQueryBuilder<T = any> { | ||
| 536 | 532 | private buildQueryParams(): URLSearchParams { |
| 537 | 533 | const params = new URLSearchParams() |
| 538 | 534 | |
| 539 | // As in BrowserModelQueryBuilder: `boolean` is not read, so an `orWhere` | |
| 540 | // serialises as an AND. Left alone deliberately — see #1096. | |
| 535 | // Every recorded where is a conjunct: orWhere() now throws rather than | |
| 536 | // recording a disjunction this format cannot express. See #1096. | |
| 541 | 537 | // Add where clauses as query params |
| 542 | 538 | for (const where of this.state.wheres) { |
| 543 | 539 | if (where.operator === '=') { |
| @@ -887,17 +883,13 @@ else { | ||
| 887 | 883 | } |
| 888 | 884 | |
| 889 | 885 | orWhere<K extends BrowserColumnName<TDef>>( |
| 890 | column: K, | |
| 891 | operatorOrValue: WhereOperator | (K extends keyof BrowserModelAttributes<TDef> ? BrowserModelAttributes<TDef>[K] : unknown), | |
| 892 | value?: K extends keyof BrowserModelAttributes<TDef> ? BrowserModelAttributes<TDef>[K] : unknown | |
| 886 | _column: K, | |
| 887 | _operatorOrValue: WhereOperator | (K extends keyof BrowserModelAttributes<TDef> ? BrowserModelAttributes<TDef>[K] : unknown), | |
| 888 | _value?: K extends keyof BrowserModelAttributes<TDef> ? BrowserModelAttributes<TDef>[K] : unknown | |
| 893 | 889 | ): BrowserModelQueryBuilder<TDef, TSelected> { |
| 894 | if (value === undefined) { | |
| 895 | this._wheres.push({ column: column as string, operator: '=', value: operatorOrValue, boolean: 'or' }) | |
| 896 | } | |
| 897 | else { | |
| 898 | this._wheres.push({ column: column as string, operator: operatorOrValue as WhereOperator, value, boolean: 'or' }) | |
| 899 | } | |
| 900 | return this | |
| 890 | // See BrowserQueryBuilder.orWhere: the recorded `boolean` was never read, | |
| 891 | // so a disjunction was transmitted as a conjunction. #1096. | |
| 892 | throw new TypeError('[browser] orWhere() is not supported by this client. The query-param format it builds has no spelling for a disjunction, so the term would be sent as a plain filter and the server would read it as AND \u2014 silently returning a superset of the rows you asked for. Express the alternatives as a single filter (whereIn), or issue one request per branch and merge the results.') | |
| 901 | 893 | } |
| 902 | 894 | |
| 903 | 895 | whereIn<K extends BrowserColumnName<TDef>>( |
| @@ -985,11 +977,9 @@ else { | ||
| 985 | 977 | private buildQueryParams(): URLSearchParams { |
| 986 | 978 | const params = new URLSearchParams() |
| 987 | 979 | |
| 988 | // `where.boolean` is not read here, so an `orWhere` serialises exactly like | |
| 989 | // a `where` and the server reads it as AND. Representing OR needs a wire | |
| 990 | // format that this file has no precedent for, so it is left alone rather | |
| 991 | // than invented — see #1096. BrowserQueryBuilder.buildQueryParams has the | |
| 992 | // same gap. | |
| 980 | // Every recorded where is a conjunct, which is what this format can say. | |
| 981 | // orWhere() throws rather than recording a disjunction that would be | |
| 982 | // transmitted — and read by the server — as one more AND term. See #1096. | |
| 993 | 983 | for (const where of this._wheres) { |
| 994 | 984 | if (where.operator === '=') { |
| 995 | 985 | params.append(where.column, String(where.value)) |