yandex-ui / noscript

Noscript: JavaScript MVC Framework for building SPA
http://yandex-ui.github.io/noscript/
MIT License
34 stars 22 forks source link

[ns.router] Тест на ? в значении параметра path #648

Closed vitkarpov closed 7 years ago

vitkarpov commented 7 years ago

Второй подход к снаряду. Как выяснил в прошлый раз, ns.router работает правильно, потому что на вход поступает заэкранированная строка (ns.router используется для парсинга урлов, поэтому очевидно там может быть не любая строка, а валидный урл).

Однако, обратно преобразование через ns.router.generateUrl не работает — лейаут не находится.

vitkarpov commented 7 years ago

Кажется, проблема в _isParamValid, которым проверяем соответствие значение параметра его типу.

Есть вот такая регулярка для path

ns.router.regexps.path = '(?:\\/[^\\/\\?]+)+';

Вот такое значение /disk/ngi?nx не заматчится на всю строку из-за ? — в итоге, нужный лейаут не нашелся. На вход параметры поступают неэкранированными, что логично.

@chestozo @alexeyten пока не уверен как это правильно чинить, что думаете?

vitkarpov commented 7 years ago

Причем на проекте это чинится сменой регулярки на \/.+? (кстати, которая кажется более логичной для матчинга на путь — может быть все, что угодно со слешем в начале, ну да ладно).

Все равно кажется странным, что ты можешь на проекте определить такую регулярку, которая разломает generateUrl, верно?

alexeyten commented 7 years ago

Мне кажется, что это из-за шизофрении регулярки. Она используется и для доставания параметра в заэнкоженном виде из URL и для валидации параметра без энкодинга.

vitkarpov commented 7 years ago

Она используется и для доставания параметра в заэнкоженном виде из URL и для валидации параметра без энкодинга.

Согласен. Надо привести к одному виду:

km256 commented 7 years ago

На вход параметры поступают неэкранированными, что логично.

Кажется, это наоборот не логично. Почему ты считаешь, что это ок?

либо экнодить значения параметров перед тем как проверять в _isParamValid

ИМХО, вот это правильный путь

vitkarpov commented 7 years ago

На вход параметры поступают неэкранированными, что логично.

Почему ты считаешь, что это ок?

Ты, наверное, не так меня понял. Я имею ввиду на вход АПИ:

generateUrl('my-layout', { p1: 'foo', p2: 'bar?#_%' })

В этом и смысл — инкапсулировать всю эту сложность с энкодингами, чтобы я не думал про это при генерации урлов снаружи.

vitkarpov commented 7 years ago

Ты, наверное, не так меня понял.

Точнее я неверно написал, в отрыве от контекста :)

i2r commented 7 years ago

Энкодинг параметра внутри _isParamValid() перед тестом регуляркой кажется логичным.

i2r commented 7 years ago

Где-то тут: https://yandex-ui.github.io/noscript/single-page/#index_md_ns_router_regexps_object надо описать, как используются регулярки типов.

vitkarpov commented 7 years ago

А что описывать? Можно любые регулярки использовать с точки зрения внешнего пользователя.

vitkarpov commented 7 years ago

@alexeyten @km256 done. Собственно перенес вызов ns.router.encodeURIComponent выше, до вызова _isParamValid.

alexeyten commented 7 years ago

Вроде ОК

chestozo commented 7 years ago

Мы обсуждали это с @i2r перенос pvalue = ns.router.encodeParamValue(pvalue, param.name); до if (!ns.router._isParamValid(pvalue, param.type) довольно критичное изменение и кажется даже нелогичным.

Давайте ещё подумаем?

vitkarpov commented 7 years ago

Давайте ещё подумаем?

Вообще, проблема именно в итоговой регулярке, которая генерит ns.router.compile. Собственно, вот https://regex101.com/r/lNaWCr/2

chestozo commented 7 years ago

Не понимаю, почему вы посчитали encoding до валидации логичным. В коде везде мы работает с разэнкоженным значением валидацию пишем для разэнкоженного значения Валидация ИМХО также должна быть того значения, с которым работаем в коде, а не того, что прилетело из урла.

vitkarpov commented 7 years ago

Можно, конечно, поменять ее, но эта проблема сейчас воспроизводится именно на нашем кастомной path и именно когда он стоит последний в шаблоне урла :)

chestozo commented 7 years ago

Пример: у меня есть тип параметра ex формата 1200,0,0 я бы хотел писать регулярку для такой строки, а не для 1200%2C0%2C0

alexeyten commented 7 years ago

Тогда этот параметр не выпарсится из урла, потому что в урле запятая будет заэнкожена.

chestozo commented 7 years ago

Да, проблема есть:

ns.router.regexps['test-regexp'] = '\\d{5},0,0';
ns.router.routes = {
    route: {
        '/test/{test-val:test-regexp}': 'test'
    }
};

в итоге

image

chestozo commented 7 years ago

По идее тогда предложенный фикс подходит, но получается все регулярки для типов ns.router.regexps надо писать с учётом этого момента (что ни будут применяться к заэнкоженному значению). Может в доке дописать про это?

vitkarpov commented 7 years ago

Может в доке дописать про это?

Да, нормальная тема. Давай здесь же отдельным коммитом добавлю тогда.

vitkarpov commented 7 years ago

DONE

chestozo commented 7 years ago

👍