Fix URL encoding (#2899)

* Fix URL encoding
* Optimize nullable criterion encoding

ui/v2.5/src/components/FrontPage/Control.tsx

@@ -107,7 +107,7 @@ const SavedFilterResults: React.FC<ISavedFilterResults> = ({
 
     const ret = new ListFilterModel(mode);
     ret.currentPage = 1;
-    ret.configureFromQueryParameters(JSON.parse(filterJSON));
+    ret.configureFromJSON(filterJSON);
     ret.randomSeed = -1;
     return ret;
   }, [data?.findSavedFilter]);

ui/v2.5/src/components/List/SavedFilterList.tsx

@@ -66,7 +66,6 @@ export const SavedFilterList: React.FC<ISavedFilterListProps> = ({
 
   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<ISavedFilterListProps> = ({
             id,
             mode: filter.mode,
             name,
-            filter: JSON.stringify(filterCopy.getSavedQueryParameters()),
+            filter: filterCopy.makeSavedFilterJSON(),
           },
         },
       });
@@ -136,7 +135,6 @@ export const SavedFilterList: React.FC<ISavedFilterListProps> = ({
 
   async function onSetDefaultFilter() {
     const filterCopy = filter.clone();
-    filterCopy.currentPage = 1;
 
     try {
       setSaving(true);
@@ -145,7 +143,7 @@ export const SavedFilterList: React.FC<ISavedFilterListProps> = ({
         variables: {
           input: {
             mode: filter.mode,
-            filter: JSON.stringify(filterCopy.getSavedQueryParameters()),
+            filter: filterCopy.makeSavedFilterJSON(),
           },
         },
       });
@@ -165,7 +163,7 @@ export const SavedFilterList: React.FC<ISavedFilterListProps> = ({
   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;
 

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))

ui/v2.5/src/hooks/ListHook.tsx

@@ -572,13 +572,14 @@ const useList = <QueryResult extends IQueryResult, QueryData extends IDataItem>(
   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 = <QueryResult extends IQueryResult, QueryData extends IDataItem>(
         if (defaultFilter?.findDefaultFilter) {
           newFilter.currentPage = 1;
           try {
-            newFilter.configureFromQueryParameters(
+            newFilter.configureFromJSON(defaultFilter.findDefaultFilter.filter);
-              JSON.parse(defaultFilter.findDefaultFilter.filter)
-            );
           } catch (err) {
             console.log(err);
             // ignore
@@ -713,9 +712,7 @@ const useList = <QueryResult extends IQueryResult, QueryData extends IDataItem>(
 
     setFilter((prevFilter) => {
       let newFilter = prevFilter.clone();
-      newFilter.configureFromQueryParameters(
+      newFilter.configureFromQueryString(location.search);
-        queryString.parse(location.search)
-      );
       if (!isEqual(newFilter, prevFilter)) {
         return newFilter;
       } else {

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<V extends CriterionValue> {
 
   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<V extends CriterionValue> {
     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 = {
+    let encodedCriterion;
-      type: this.criterionOption.type,
+    if (
-      // #394 - the presence of a # symbol results in the query URL being
+      this.modifier === CriterionModifier.IsNull ||
-      // malformed. We could set encode: true in the queryString.stringify
+      this.modifier === CriterionModifier.NotNull
-      // call below, but this results in a URL that gets pretty long and ugly.
+    ) {
-      // Instead, we'll encode the criteria values.
+      encodedCriterion = {
-      value: this.encodeValue(),
+        type: this.criterionOption.type,
-      modifier: this.modifier,
+        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<string> {
-  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<INumberValue> {
-  private getValue() {
+  public get value(): INumberValue {
-    // backwards compatibility - if this.value is a number, use that
+    return this._value;
-    if (typeof this.value !== "object") {
-      return this.value as number;
-    }
-
-    return this.value.value;
   }
-
+  public set value(newValue: number | INumberValue) {
-  public encodeValue() {
+    // backwards compatibility - if this.value is a number, use that
-    return {
+    if (typeof newValue !== "object") {
-      value: this.getValue(),
+      this._value = {
-      value2: this.value.value2,
+        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();
+    const { value, value2 } = this.value;
-    return this.modifier === CriterionModifier.Between ||
+    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<ILabeledId[]> {
     };
   }
 
-  public encodeValue() {
-    return this.value.map((o) => {
-      return encodeILabeledId(o);
-    });
-  }
-
   constructor(type: CriterionOption) {
     super(type, []);
   }
 }
 
 export class IHierarchicalLabeledIdCriterion extends Criterion<IHierarchicalLabelValue> {
-  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<IHierarchicalLabe
   }
 
   public getLabelValue(): string {
-    const labels = decodeURI(
+    const labels = (this.value.items ?? []).map((v) => v.label).join(", ");
-      (this.value.items ?? []).map((v) => encodeLabel(v.label)).join(", ")
-    );
 
     if (this.value.depth === 0) {
       return labels;
@@ -509,13 +470,6 @@ export class DurationCriterion extends Criterion<INumberValue> {
     super(type, { value: 0, value2: undefined });
   }
 
-  public encodeValue() {
-    return {
-      value: this.value.value,
-      value2: this.value.value2,
-    };
-  }
-
   protected toCriterionInput(): IntCriterionInput {
     return {
       modifier: this.modifier,

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<string>,
     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[];
+      params.c.forEach((jsonString) => {
-      if (params.c instanceof Array) {
-        jsonParameters = params.c;
-      } else {
-        jsonParameters = [params.c];
-      }
-
-      jsonParameters.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<string>
+  ): 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() {
+  // Returns query parameters with necessary parts encoded
-    const encodedCriteria: string[] = this.criteria.map((criterion) =>
+  public getQueryParameters(): IQueryParameters {
-      criterion.toJSON()
+    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 {

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;

ui/v2.5/src/models/sceneQueue.ts

@@ -6,15 +6,6 @@ import { SceneListFilterOptions } from "./list-filter/scenes";
 
 export type QueuedScene = Pick<Scene, "id" | "title" | "paths">;
 
-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