kadirahq / flow-router

Carefully Designed Client Side Router for Meteor
MIT License
1.09k stars 195 forks source link

URL param double encode issue #581

Open imajus opened 8 years ago

imajus commented 8 years ago

I've found out that when I'm trying to use russian symbols in route params the FlowRouter not only encodes them internally but does that twice. For example if I try to build an URL like FlowRouter.path('news', { tag: 'авто' }) it will return something like /news/%25D0%25B0%25D0%25B2%25D1%2582%25D0%25BE not /news/авто as I expect it.

Here's a part in FlowRouter.path definition:

    // this is to allow page js to keep the custom characters as it is
    // we need to encode 2 times otherwise "/" char does not work properly
    // So, in that case, when I includes "/" it will think it's a part of the
    // route. encoding 2times fixes it
    return encodeURIComponent(encodeURIComponent(fields[key] || ""));

I didn't get the comment here so I don't understand the reason of such behavior. IronRouter handled russian symbols normally so I guess there's way to do that in FlowRouter too.

This is kind of major issue for all non-english websites because good SEO depends on content in URLs.

JH7 commented 4 years ago

In case someone is still interested in this bug: Insert these lines in a new .js file which is imported on the client side. Make sure that the file is imported as one of the first files especially before your first usage of FlowRouter or anything related (e. g. AccountsTemplates).

import { FlowRouter } from 'meteor/kadira:flow-router';

FlowRouter._page.Route.prototype.match = function(path, params) {
  const { keys } = this;
  const qsIndex = path.indexOf('?');
  const pathname = ~qsIndex ? path.slice(0, qsIndex) : path;
  const m = this.regexp.exec(decodeURIComponent(pathname));

  if (!m) return false;

  for (let i = 1, len = m.length; i < len; ++i) {
    const key = keys[i - 1];
    const val = m[i];
    if (val !== undefined || !(hasOwnProperty.call(params, key.name))) {
      params[key.name] = val;
    }
  }

  return true;
};

FlowRouter.Router.prototype.path = function(pathDef, fields, queryParams) {
  let pathDefToUse = pathDef;
  let fieldsToUse = fields;

  if (FlowRouter._routesMap[pathDefToUse]) {
    pathDefToUse = FlowRouter._routesMap[pathDefToUse].pathDef;
  }

  let path = '';

  // Prefix the path with the router global prefix
  if (FlowRouter._basePath) {
    path += `/${FlowRouter._basePath}/`;
  }

  fieldsToUse = fieldsToUse || {};
  const regExp = /(:[\w\(\)\\\+\*\.\?]+)+/g;
  path += pathDefToUse.replace(regExp, function(key) {
    let keyToUse = key;

    const firstRegexpChar = keyToUse.indexOf('(');
    // get the content behind : and (\\d+/)
    keyToUse = keyToUse.substring(1, (firstRegexpChar > 0) ? firstRegexpChar : undefined);
    // remove +?*
    keyToUse = keyToUse.replace(/[\+\*\?]+/g, '');

    // this is to allow page js to keep the custom characters as it is
    // we need to encode 2 times otherwise "/" char does not work properly
    // So, in that case, when I includes "/" it will think it's a part of the
    // route. encoding 2times fixes it
    return encodeURIComponent(fieldsToUse[keyToUse] || '');
  });

  // Replace multiple slashes with single slash
  path = path.replace(/\/\/+/g, '/');

  // remove trailing slash
  // but keep the root slash if it's the only one
  path = path.match(/^\/{1}$/) ? path : path.replace(/\/$/, '');

  // explictly asked to add a trailing slash
  if (FlowRouter.env.trailingSlash.get() && _.last(path) !== '/') {
    path += '/';
  }

  const strQueryParams = FlowRouter._qs.stringify(queryParams || {});
  if (strQueryParams) {
    path += `?${strQueryParams}`;
  }

  return path;
};

A dirty hack which works out quite well (so far). This fixes the double encodeURIComponent within FlowRouter.path but also a double decoding within pagejs.match. Please keep in mind that I do not fully understand how its working and therefore cannot guarantee that this will work out for you too.