diff --git a/ui/v2.5/src/components/FrontPage/Control.tsx b/ui/v2.5/src/components/FrontPage/Control.tsx index 019429b3d..4175d30d5 100644 --- a/ui/v2.5/src/components/FrontPage/Control.tsx +++ b/ui/v2.5/src/components/FrontPage/Control.tsx @@ -107,7 +107,7 @@ const SavedFilterResults: React.FC = ({ const ret = new ListFilterModel(mode); ret.currentPage = 1; - ret.configureFromQueryParameters(JSON.parse(filterJSON)); + ret.configureFromJSON(filterJSON); ret.randomSeed = -1; return ret; }, [data?.findSavedFilter]); diff --git a/ui/v2.5/src/components/List/SavedFilterList.tsx b/ui/v2.5/src/components/List/SavedFilterList.tsx index 732fd141f..87be25365 100644 --- a/ui/v2.5/src/components/List/SavedFilterList.tsx +++ b/ui/v2.5/src/components/List/SavedFilterList.tsx @@ -66,7 +66,6 @@ export const SavedFilterList: React.FC = ({ async function onSaveFilter(name: string, id?: string) { const filterCopy = filter.clone(); - filterCopy.currentPage = 1; try { setSaving(true); @@ -76,7 +75,7 @@ export const SavedFilterList: React.FC = ({ id, mode: filter.mode, name, - filter: JSON.stringify(filterCopy.getSavedQueryParameters()), + filter: filterCopy.makeSavedFilterJSON(), }, }, }); @@ -136,7 +135,6 @@ export const SavedFilterList: React.FC = ({ async function onSetDefaultFilter() { const filterCopy = filter.clone(); - filterCopy.currentPage = 1; try { setSaving(true); @@ -145,7 +143,7 @@ export const SavedFilterList: React.FC = ({ variables: { input: { mode: filter.mode, - filter: JSON.stringify(filterCopy.getSavedQueryParameters()), + filter: filterCopy.makeSavedFilterJSON(), }, }, }); @@ -165,7 +163,7 @@ export const SavedFilterList: React.FC = ({ function filterClicked(f: SavedFilterDataFragment) { const newFilter = filter.clone(); newFilter.currentPage = 1; - newFilter.configureFromQueryParameters(JSON.parse(f.filter)); + newFilter.configureFromJSON(f.filter); // #1507 - reset random seed when loaded newFilter.randomSeed = -1; diff --git a/ui/v2.5/src/docs/en/Changelog/v0170.md b/ui/v2.5/src/docs/en/Changelog/v0170.md index 492d4cfe7..104b28e92 100644 --- a/ui/v2.5/src/docs/en/Changelog/v0170.md +++ b/ui/v2.5/src/docs/en/Changelog/v0170.md @@ -11,6 +11,7 @@ After migrating, please run a scan on your entire library to populate missing da * Added release notes dialog. ([#2726](https://github.com/stashapp/stash/pull/2726)) ### 🎨 Improvements +* Encode reserved characters in query URLs. ([#2899](https://github.com/stashapp/stash/pull/2899)) * Object titles are now displayed as the file basename if the title is not explicitly set. The `Don't include file extension as part of the title` scan flag is no longer supported. * `Set name, date, details from embedded file metadata` scan flag is no longer supported. This functionality may be implemented as a built-in scraper in the future. * Moved Changelogs to Settings page. ([#2726](https://github.com/stashapp/stash/pull/2726)) diff --git a/ui/v2.5/src/hooks/ListHook.tsx b/ui/v2.5/src/hooks/ListHook.tsx index 732610096..23d52f966 100644 --- a/ui/v2.5/src/hooks/ListHook.tsx +++ b/ui/v2.5/src/hooks/ListHook.tsx @@ -572,13 +572,14 @@ const useList = ( const defaultSort = options.defaultSort ?? filterOptions.defaultSortBy; const defaultDisplayMode = filterOptions.displayModeOptions[0]; const createNewFilter = useCallback(() => { - return new ListFilterModel( + const filter = new ListFilterModel( options.filterMode, - queryString.parse(history.location.search), defaultSort, defaultDisplayMode, options.defaultZoomIndex ); + filter.configureFromQueryString(history.location.search); + return filter; }, [ options.filterMode, history, @@ -654,9 +655,7 @@ const useList = ( if (defaultFilter?.findDefaultFilter) { newFilter.currentPage = 1; try { - newFilter.configureFromQueryParameters( - JSON.parse(defaultFilter.findDefaultFilter.filter) - ); + newFilter.configureFromJSON(defaultFilter.findDefaultFilter.filter); } catch (err) { console.log(err); // ignore @@ -713,9 +712,7 @@ const useList = ( setFilter((prevFilter) => { let newFilter = prevFilter.clone(); - newFilter.configureFromQueryParameters( - queryString.parse(location.search) - ); + newFilter.configureFromQueryString(location.search); if (!isEqual(newFilter, prevFilter)) { return newFilter; } else { diff --git a/ui/v2.5/src/models/list-filter/criteria/criterion.ts b/ui/v2.5/src/models/list-filter/criteria/criterion.ts index 23ef0ba5c..7597874a1 100644 --- a/ui/v2.5/src/models/list-filter/criteria/criterion.ts +++ b/ui/v2.5/src/models/list-filter/criteria/criterion.ts @@ -11,8 +11,6 @@ import { import DurationUtils from "src/utils/duration"; import { CriterionType, - encodeILabeledId, - encodeLabel, IHierarchicalLabelValue, ILabeledId, ILabeledValue, @@ -54,7 +52,14 @@ export abstract class Criterion { public criterionOption: CriterionOption; public modifier: CriterionModifier; - public value: V; + + protected _value!: V; + public get value(): V { + return this._value; + } + public set value(newValue: V) { + this._value = newValue; + } public abstract getLabelValue(): string; @@ -97,24 +102,23 @@ export abstract class Criterion { return `${this.criterionOption.parameterName}-${this.modifier.toString()}`; // TODO add values? } - public encodeValue(): V { - return this.value; - } - - public decodeValue(value: V) { - this.value = value; - } - public toJSON() { - const encodedCriterion = { - type: this.criterionOption.type, - // #394 - the presence of a # symbol results in the query URL being - // malformed. We could set encode: true in the queryString.stringify - // call below, but this results in a URL that gets pretty long and ugly. - // Instead, we'll encode the criteria values. - value: this.encodeValue(), - modifier: this.modifier, - }; + let encodedCriterion; + if ( + this.modifier === CriterionModifier.IsNull || + this.modifier === CriterionModifier.NotNull + ) { + encodedCriterion = { + type: this.criterionOption.type, + modifier: this.modifier, + }; + } else { + encodedCriterion = { + type: this.criterionOption.type, + value: this.value, + modifier: this.modifier, + }; + } return JSON.stringify(encodedCriterion); } @@ -207,32 +211,13 @@ export function createStringCriterionOption( } export class StringCriterion extends Criterion { - public getLabelValue() { - let ret = this.value; - ret = StringCriterion.unreplaceSpecialCharacter(ret, "&"); - ret = StringCriterion.unreplaceSpecialCharacter(ret, "+"); - return ret; - } - - public encodeValue() { - // replace certain characters - let ret = this.value; - ret = StringCriterion.replaceSpecialCharacter(ret, "&"); - ret = StringCriterion.replaceSpecialCharacter(ret, "+"); - return ret; - } - - private static replaceSpecialCharacter(str: string, c: string) { - return str.replaceAll(c, encodeURIComponent(c)); - } - - private static unreplaceSpecialCharacter(str: string, c: string) { - return str.replaceAll(encodeURIComponent(c), c); - } - constructor(type: CriterionOption) { super(type, ""); } + + public getLabelValue() { + return this.value; + } } export class MandatoryStringCriterionOption extends CriterionOption { @@ -337,37 +322,39 @@ export function createNumberCriterionOption(value: CriterionType) { } export class NumberCriterion extends Criterion { - private getValue() { - // backwards compatibility - if this.value is a number, use that - if (typeof this.value !== "object") { - return this.value as number; - } - - return this.value.value; + public get value(): INumberValue { + return this._value; } - - public encodeValue() { - return { - value: this.getValue(), - value2: this.value.value2, - }; + public set value(newValue: number | INumberValue) { + // backwards compatibility - if this.value is a number, use that + if (typeof newValue !== "object") { + this._value = { + value: newValue, + value2: undefined, + }; + } else { + this._value = newValue; + } } protected toCriterionInput(): IntCriterionInput { - // backwards compatibility - if this.value is a number, use that return { modifier: this.modifier, - value: this.getValue(), + value: this.value.value, value2: this.value.value2, }; } public getLabelValue() { - const value = this.getValue(); - return this.modifier === CriterionModifier.Between || + const { value, value2 } = this.value; + if ( + this.modifier === CriterionModifier.Between || this.modifier === CriterionModifier.NotBetween - ? `${value}, ${this.value.value2 ?? 0}` - : `${value}`; + ) { + return `${value}, ${value2 ?? 0}`; + } else { + return `${value}`; + } } constructor(type: CriterionOption) { @@ -417,36 +404,12 @@ export class ILabeledIdCriterion extends Criterion { }; } - public encodeValue() { - return this.value.map((o) => { - return encodeILabeledId(o); - }); - } - constructor(type: CriterionOption) { super(type, []); } } export class IHierarchicalLabeledIdCriterion extends Criterion { - public encodeValue() { - return { - items: this.value.items.map((o) => { - return encodeILabeledId(o); - }), - depth: this.value.depth, - }; - } - - // eslint-disable-next-line @typescript-eslint/no-explicit-any - public decodeValue(value: any) { - if (Array.isArray(value)) { - this.value.items = value; - } else { - this.value = value; - } - } - protected toCriterionInput(): HierarchicalMultiCriterionInput { return { value: (this.value.items ?? []).map((v) => v.id), @@ -456,9 +419,7 @@ export class IHierarchicalLabeledIdCriterion extends Criterion encodeLabel(v.label)).join(", ") - ); + const labels = (this.value.items ?? []).map((v) => v.label).join(", "); if (this.value.depth === 0) { return labels; @@ -509,13 +470,6 @@ export class DurationCriterion extends Criterion { super(type, { value: 0, value2: undefined }); } - public encodeValue() { - return { - value: this.value.value, - value2: this.value.value2, - }; - } - protected toCriterionInput(): IntCriterionInput { return { modifier: this.modifier, diff --git a/ui/v2.5/src/models/list-filter/filter.ts b/ui/v2.5/src/models/list-filter/filter.ts index c1c5c4d5b..b3ff10575 100644 --- a/ui/v2.5/src/models/list-filter/filter.ts +++ b/ui/v2.5/src/models/list-filter/filter.ts @@ -1,4 +1,5 @@ import queryString, { ParsedQuery } from "query-string"; +import clone from "lodash-es/clone"; import { FilterMode, FindFilterType, @@ -42,26 +43,24 @@ export class ListFilterModel { public constructor( mode: FilterMode, - rawParms?: ParsedQuery, defaultSort?: string, defaultDisplayMode?: DisplayMode, defaultZoomIndex?: number ) { this.mode = mode; - const params = rawParms as IQueryParameters; this.sortBy = defaultSort; if (defaultDisplayMode !== undefined) this.displayMode = defaultDisplayMode; if (defaultZoomIndex !== undefined) { this.defaultZoomIndex = defaultZoomIndex; this.zoomIndex = defaultZoomIndex; } - if (params) this.configureFromQueryParameters(params); } public clone() { return Object.assign(new ListFilterModel(this.mode), this); } + // Does not decode any URL-encoding in parameters public configureFromQueryParameters(params: IQueryParameters) { if (params.sortby !== undefined) { this.sortBy = params.sortby; @@ -102,20 +101,15 @@ export class ListFilterModel { this.criteria = []; if (params.c !== undefined) { - let jsonParameters: string[]; - if (params.c instanceof Array) { - jsonParameters = params.c; - } else { - jsonParameters = [params.c]; - } - - jsonParameters.forEach((jsonString) => { + params.c.forEach((jsonString) => { try { const encodedCriterion = JSON.parse(jsonString); const criterion = makeCriteria(encodedCriterion.type); // it's possible that we have unsupported criteria. Just skip if so. if (criterion) { - criterion.decodeValue(encodedCriterion.value); + if (encodedCriterion.value !== undefined) { + criterion.value = encodedCriterion.value; + } criterion.modifier = encodedCriterion.modifier; this.criteria.push(criterion); } @@ -127,6 +121,49 @@ export class ListFilterModel { } } + public static decodeQueryParameters( + parsedQuery: ParsedQuery + ): IQueryParameters { + const params = clone(parsedQuery); + if (params.q) { + let searchTerm: string; + if (params.q instanceof Array) { + searchTerm = params.q[0]; + } else { + searchTerm = params.q; + } + params.q = decodeURIComponent(searchTerm); + } + if (params.c !== undefined) { + let jsonParameters: string[]; + if (params.c instanceof Array) { + jsonParameters = params.c; + } else { + jsonParameters = [params.c!]; + } + params.c = jsonParameters.map((jsonString) => { + let decodedJson = jsonString; + // replace () back to {} + decodedJson = decodedJson.replaceAll("(", "{"); + decodedJson = decodedJson.replaceAll(")", "}"); + // decode all other characters + decodedJson = decodeURIComponent(decodedJson); + return decodedJson; + }); + } + return params; + } + + public configureFromQueryString(query: string) { + const parsed = queryString.parse(query, { decode: false }); + const decoded = ListFilterModel.decodeQueryParameters(parsed); + this.configureFromQueryParameters(decoded); + } + + public configureFromJSON(json: string) { + this.configureFromQueryParameters(JSON.parse(json)); + } + private setRandomSeed() { if (this.sortBy === "random") { // #321 - set the random seed if it is not set @@ -149,36 +186,55 @@ export class ListFilterModel { return this.sortBy; } - public getQueryParameters() { - const encodedCriteria: string[] = this.criteria.map((criterion) => - criterion.toJSON() - ); + // Returns query parameters with necessary parts encoded + public getQueryParameters(): IQueryParameters { + const encodedCriteria: string[] = this.criteria.map((criterion) => { + let str = criterion.toJSON(); + // URL-encode other characters + str = encodeURI(str); + // force URL-encode existing () + str = str.replaceAll("(", "%28"); + str = str.replaceAll(")", "%29"); + // replace JSON '{'(%7B) '}'(%7D) with explicitly unreserved () + str = str.replaceAll("%7B", "("); + str = str.replaceAll("%7D", ")"); + // only the reserved characters ?#&;=+ need to be URL-encoded + // as they have special meaning in query strings + str = str.replaceAll("?", encodeURIComponent("?")); + str = str.replaceAll("#", encodeURIComponent("#")); + str = str.replaceAll("&", encodeURIComponent("&")); + str = str.replaceAll(";", encodeURIComponent(";")); + str = str.replaceAll("=", encodeURIComponent("=")); + str = str.replaceAll("+", encodeURIComponent("+")); + return str; + }); - const result = { + return { perPage: this.itemsPerPage !== DEFAULT_PARAMS.itemsPerPage - ? this.itemsPerPage + ? String(this.itemsPerPage) : undefined, sortby: this.getSortBy() ?? undefined, sortdir: this.sortDirection === SortDirectionEnum.Desc ? "desc" : undefined, disp: this.displayMode !== DEFAULT_PARAMS.displayMode - ? this.displayMode + ? String(this.displayMode) : undefined, q: this.searchTerm ? encodeURIComponent(this.searchTerm) : undefined, p: this.currentPage !== DEFAULT_PARAMS.currentPage - ? this.currentPage + ? String(this.currentPage) + : undefined, + z: + this.zoomIndex !== this.defaultZoomIndex + ? String(this.zoomIndex) : undefined, - z: this.zoomIndex !== this.defaultZoomIndex ? this.zoomIndex : undefined, c: encodedCriteria, }; - - return result; } - public getSavedQueryParameters() { + public makeSavedFilterJSON() { const encodedCriteria: string[] = this.criteria.map((criterion) => criterion.toJSON() ); @@ -194,7 +250,7 @@ export class ListFilterModel { c: encodedCriteria, }; - return result; + return JSON.stringify(result); } public makeQueryParameters(): string { diff --git a/ui/v2.5/src/models/list-filter/types.ts b/ui/v2.5/src/models/list-filter/types.ts index f97a48446..46313a314 100644 --- a/ui/v2.5/src/models/list-filter/types.ts +++ b/ui/v2.5/src/models/list-filter/types.ts @@ -47,16 +47,6 @@ export function criterionIsNumberValue( return typeof value === "object" && "value" in value && "value2" in value; } -export function encodeLabel(v: string) { - // escape " and \ and by encoding to JSON so that it encodes to JSON correctly down the line - const adjustedLabel = JSON.stringify(v).slice(1, -1); - return encodeURIComponent(adjustedLabel); -} - -export function encodeILabeledId(o: ILabeledId) { - return { ...o, label: encodeLabel(o.label) }; -} - export interface IOptionType { id: string; name?: string; diff --git a/ui/v2.5/src/models/sceneQueue.ts b/ui/v2.5/src/models/sceneQueue.ts index 0eb4ac6fd..199218e33 100644 --- a/ui/v2.5/src/models/sceneQueue.ts +++ b/ui/v2.5/src/models/sceneQueue.ts @@ -6,15 +6,6 @@ import { SceneListFilterOptions } from "./list-filter/scenes"; export type QueuedScene = Pick; -interface IQueryParameters { - qsort?: string; - qsortd?: string; - qfq?: string; - qfp?: string; - qfc?: string[]; - qs?: string[]; -} - export interface IPlaySceneOptions { sceneIndex?: number; newPage?: number; @@ -88,7 +79,7 @@ export class SceneQueue { public static fromQueryParameters(params: string) { const ret = new SceneQueue(); - const parsed = queryString.parse(params) as IQueryParameters; + const parsed = queryString.parse(params, { decode: false }); const translated = { sortby: parsed.qsort, sortdir: parsed.qsortd, @@ -98,11 +89,12 @@ export class SceneQueue { }; if (parsed.qfp) { + const decoded = ListFilterModel.decodeQueryParameters(translated); const query = new ListFilterModel( FilterMode.Scenes, - translated as queryString.ParsedQuery, SceneListFilterOptions.defaultSortBy ); + query.configureFromQueryParameters(decoded); ret.query = query; } else if (parsed.qs) { // must be scene list