feat: added redirectIgnoresNonGetMethods and stringify options (closes …

…#10, closes ladjs/lad#372)

README.md

@@ -95,6 +95,8 @@ It also sets the cookie `locale` for future requests to their detected locale.
 
 This also stores the `last_locale` (or whatever you configure the property name to be in the config option `lastLocaleField`) for a user via `ctx.state.user.save()`.
 
+**NOTE:** As of v2.0.0 we have added a `redirectIgnoresNonGetMethods` (Boolean) option (defaults to `true`) which you can pass to `new I18N({ ... })` which will ignore non-GET methods on redirection.
+
 
 ## Options
 
@@ -126,7 +128,14 @@ const i18n = new I18N({
   },
   register: i18n.api,
   lastLocaleField: 'last_locale',
-  ignoredRedirectGlobs: []
+  ignoredRedirectGlobs: [],
+  redirectIgnoresNonGetMethods: true,
+  // <https://github.com/ljharb/qs>
+  stringify: {
+    addQueryPrefix: true,
+    format: 'RFC1738',
+    arrayFormat: 'indices'
+  }
 });
 ```
 

src/index.js

@@ -38,7 +38,13 @@ class I18N {
         },
         register: i18n.api,
         lastLocaleField: 'last_locale',
-        ignoredRedirectGlobs: []
+        ignoredRedirectGlobs: [],
+        redirectIgnoresNonGetMethods: true,
+        stringify: {
+          addQueryPrefix: true,
+          format: 'RFC1738',
+          arrayFormat: 'indices'
+        }
       },
       config
     );
@@ -133,7 +139,9 @@ class I18N {
       debug('locale was not available redirecting user');
       return ctx.redirect(
         `/${ctx.state.locale}${ctx.pathWithoutLocale}${
-          isEmpty(ctx.query) ? '' : `?${stringify(ctx.query)}`
+          isEmpty(ctx.query)
+            ? ''
+            : `?${stringify(ctx.query, this.config.stringify)}`
         }`
       );
     }
@@ -144,7 +152,9 @@ class I18N {
         return {
           locale,
           url: `/${locale}${ctx.pathWithoutLocale}${
-            isEmpty(ctx.query) ? '' : `?${stringify(ctx.query)}`
+            isEmpty(ctx.query)
+              ? ''
+              : `?${stringify(ctx.query, this.config.stringify)}`
           }`,
           name: getLanguage(locale).name[0]
         };
@@ -178,6 +188,10 @@ class I18N {
     // do not redirect static paths
     if (extname(ctx.path) !== '') return next();
 
+    // if the method is not a GET request then ignore it
+    if (this.config.redirectIgnoresNonGetMethods && ctx.method !== 'GET')
+      return next();
+
     // check against ignored/whitelisted redirect middleware paths
     const match = multimatch(ctx.path, this.config.ignoredRedirectGlobs);
     if (Array.isArray(match) && match.length > 0) {
@@ -194,10 +208,11 @@ class I18N {
     // then redirect the user to their detected locale
     if (!hasLang) {
       ctx.status = 302;
-      let redirect = `/${ctx.request.locale}${ctx.url}`;
+      let redirect = `/${ctx.request.locale}${ctx.path}`;
       if (redirect === `/${ctx.request.locale}/`)
         redirect = `/${ctx.request.locale}`;
-      if (!isEmpty(ctx.query)) redirect += `?${stringify(ctx.query)}`;
+      if (!isEmpty(ctx.query))
+        redirect += `${stringify(ctx.query, this.config.stringify)}`;
       debug('no valid locale found in URL, redirecting to %s', redirect);
       return ctx.redirect(redirect);
     }

test/test.js

@@ -222,6 +222,77 @@ test('does not redirect with ignored redirect globs', async t => {
   t.is(res.headers.location, '/es/login/beep/baz');
 });
 
+// https://github.com/ladjs/lad/issues/372
+test('does not duplicate querystring if no locale provided', async t => {
+  // https://lad.sh?test=test?test=test?test=test%3Ftest%3Dtest
+  const app = new Koa();
+  const i18n = new I18N({ phrases, directory });
+
+  app.use(session());
+  app.use(i18n.middleware);
+  app.use(i18n.redirect);
+
+  app.use(ctx => {
+    const { locale } = ctx;
+    ctx.body = { locale };
+    ctx.status = 200;
+  });
+
+  const res = await request(app.listen()).get(
+    '/?test=test?test=test?test=test'
+  );
+
+  t.is(res.status, 302);
+  t.is(res.headers.location, '/en?test=test%3Ftest%3Dtest%3Ftest%3Dtest');
+});
+
+test('redirectIgnoresNonGetMethods', async t => {
+  const app = new Koa();
+  const i18n = new I18N({ phrases, directory });
+
+  app.use(session());
+  app.use(i18n.middleware);
+  app.use(i18n.redirect);
+
+  app.use(ctx => {
+    const { locale } = ctx;
+    ctx.body = { locale };
+    ctx.status = 200;
+  });
+
+  let res = await request(app.listen()).get('/');
+  t.is(res.status, 302);
+  t.is(res.headers.location, '/en');
+
+  res = await request(app.listen()).post('/');
+  t.is(res.status, 200);
+});
+
+test('ignoredRedirectGlobs', async t => {
+  const app = new Koa();
+  const i18n = new I18N({
+    ignoredRedirectGlobs: ['/foo', '/baz/**/*'],
+    phrases,
+    directory
+  });
+
+  app.use(session());
+  app.use(i18n.middleware);
+  app.use(i18n.redirect);
+
+  app.use(ctx => {
+    const { locale } = ctx;
+    ctx.body = { locale };
+    ctx.status = 200;
+  });
+
+  let res = await request(app.listen()).get('/foo');
+  t.is(res.status, 200);
+
+  res = await request(app.listen()).get('/baz/beep/bar');
+  t.is(res.status, 200);
+});
+
 test('redirects to correct path based on locale set via cookie', async t => {
   const app = new Koa();
   const i18n = new I18N({ phrases, directory });