Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Некорректное кодирование параметров запроса #138

Open
ChugunovAN opened this issue Apr 5, 2024 · 14 comments · May be fixed by #139
Open

Некорректное кодирование параметров запроса #138

ChugunovAN opened this issue Apr 5, 2024 · 14 comments · May be fixed by #139

Comments

@ChugunovAN
Copy link

ChugunovAN commented Apr 5, 2024

В функции КодироватьПараметрыЗапроса происходит неявное приведение к типу Строка значения параметра в вызове КодироватьСтроку
ЗначениеПараметра = КодироватьСтроку(Значение, СпособКодированияСтроки.КодировкаURL);

  1. Тип Булево: Истина, Ложь -> "Да", "Нет" (в кодировке URL) вместо "true", "false"
  2. Тип Число: 123456 -> "123 456" (Символы.НПП в кодировке URL) вместо "123456"
  3. Тип Дата: '20240131231012' -> "31.01.2024 23:10:12" (Символ : в кодировке URL) вместо "2024-01-31T23:10:12"

В случае кодирования параметров запроса для тела запроса Content-type="multipart/form-data" не требует кодировки URL, что касается и типа Строка, и других типов.

@leemuar
Copy link
Collaborator

leemuar commented Apr 10, 2024

Спасибо за PR! Но я не уверен, что готов его принять в текущей реализации

Автоматическая конвертация в строку зависит от текущей локали операционной системы, и полагаться на нее не стоит.
Я пока не уверен, что библиотека должна давать какие-то гарантии на подобное преобразование. Может булево должно быть 0 и 1? Или дата должна быть в unix time? Для каждой системы свои правила. Сейчас кажется, что решать такие вопросы - это не ответственность библиотеки

@ChugunovAN
Copy link
Author

Автоматическая конвертация в строку зависит от текущей локали операционной системы, и полагаться на нее не стоит. Я пока не уверен, что библиотека должна давать какие-то гарантии на подобное преобразование. Может булево должно быть 0 и 1? Или дата должна быть в unix time? Для каждой системы свои правила. Сейчас кажется, что решать такие вопросы - это не ответственность библиотеки

Из этой ситуации существуют два выхода:

  1. Либо обязать в передаваемых параметрах использовать только значения типа строка и возложить ответственность за корректную интерпретацию этих значений на вызывающую сторону.
  2. Либо реализовать необходимое преобразование в самом методе. Что уже очень лаконично сделал @alexandr-yang. За что ему огромное спасибо!

@alexandr-yang я в комите оставил маленькое замечание, при исправлении которого поведение станет максимально корректным и для формата multipart/form-data. Заранее буду признателен за любой ответ на этот комментарий.

@leemuar
Copy link
Collaborator

leemuar commented Apr 10, 2024

Из этой ситуации существуют два выхода:

Есть еще третий - оставить конвертацию по стандартной для платформы схеме: в соответствии с локалью операционной системы. т.е. так как работает сейчас.

Я с уважением и благодарностью отношусь к труду всех кто присылает PR, но необходимость такой конвертации самой библиотекой сейчас не достаточно аргументирована. Для разных случаев нужны разные значения, форсировать библиотекой какие-то одни я не считаю коректным. Стандартная конвертация с учетом локали - достаточно привычное поведение платформы, что передать в качестве значения - ответственность программиста.

Я приглашаю к обсуждению и аргументации

@alexandr-yang
Copy link
Contributor

XMLСтрока преобразует в строку четко, без привязки к локали. Поведение будет всегда предсказуемое и на мой взгляд очень логичное.
image

Если нужно как-то "хитро" - тут уже нужно самому преобразовать и передать в параметры строку

@alexandr-yang
Copy link
Contributor

я в комите оставил маленькое замечание

Как по мне, там логика немного сломана. Метод ПодготовитьТелоЗапроса используется не по назначению.

@leemuar
Copy link
Collaborator

leemuar commented Apr 10, 2024

XMLСтрока преобразует в строку четко, без привязки к локали. Поведение будет всегда предсказуемое и на мой взгляд очень логичное.

оно логичное только в рамках одной "предметной" области. В другой логично будет другое, я приводил выше пример с 0 и 1 как булево и unixtime. В JSON, который ближе к web, например, формат даты не стандартизирован, хотя и часто используется unixtime.

Я не могу не отметить изящество в краткости преобразования с помощью XMLСтрока, но само по себе это не аргумент в пользу такого преобразования

P.S. Есть еще другой контраргумент - на текущее поведение кто-то мог завязаться, и таким изменением мы можем сломать людям код. Да, это не документированный сайд-эффект, но все же.

@alexandr-yang
Copy link
Contributor

Что-то сломаться может, тут согласен. В остальном нет. Преобразование булева к числу - это скорее всего костыль, как со стороны сервера, так и со стороны клиента. В вебе чаще всего отправляют true и false. Но это все холивар.

Другой вариант, можно значение пропустить через ОбъектВJson, будет по сути то же самое. А кастомные настройки можно передать через ПараметрыПреобразования (как это сделать красиво - тоже вопрос). Вариант будет более гибкий, но проблему с совместимостью он не решит.

@leemuar
Copy link
Collaborator

leemuar commented Apr 10, 2024

Преобразование булева к числу - это скорее всего костыль, как со стороны сервера, так и со стороны клиента. В вебе чаще всего отправляют true и false.

Не холивар, нормальное обсуждение. Повторюсь, у меня нет цели отказать в PR, цель - обсудить и понять насколько это изменение приемлемо.

В вебе часто применяют и числа: redirect=1. Посмотрите, например, https://www.ibm.com/docs/en/control-desk/7.6.1.2?topic=api-rest-query-parameters Вполне четко написано: "1 для Boolean true, 0 - для false"

true и false часто появляются там где JS фреймворки, потому что такое им проще парсить в объекты JS. Но стандарта на это нет, поэтому я считаю форсировать какое-то соглашение здесь не стоит, это не обязанность библиотеки. И типовое поведение библиотеки здесь - использовать стандартную для платформы сериализацию - вполне нормально.

@alexandr-yang
Copy link
Contributor

Да я тоже не настаиваю в мерже, просто предлагаю решения на основе своего опыта. В общем, решайте. Если нужно будет сделать правки - мне не сложно)

@ChugunovAN
Copy link
Author

Выражу мнение как разработчик, использующий библиотеку: это изменение улучшит лаконичность используемых методов и для разработчика 1С такое приведение значений параметров будет ожидаемым. И можно сказать точно, что Истина/Ложь в виде строки URLEncoded ни в каких кейсах не нужны. Как и Символы.НПП как разделитель триад в числе. С приведением дат к формату yyyy-mm-dd тоже гораздо более ожидаемо, чем формат определяемый локалью.
В особых случаях же (булево 1 или 0, unixtime и т.д.) от разработчика все равно понадобится разобраться с форматом требуемым для конкретного http-сервиса. Что не ухудшит понятность подготовки параметров.

@PLebedevV
Copy link
Contributor

можно сказать точно, что Истина/Ложь в виде строки URLEncoded ни в каких кейсах не нужны. Как и Символы.НПП как разделитель триад в числе. С приведением дат к формату yyyy-mm-dd тоже гораздо более ожидаемо, чем формат определяемый локалью.

полностью согласен

@zeegin
Copy link
Contributor

zeegin commented Apr 23, 2024

Кажется что исправление пользы приносит больше чес вреда.

То что коннектор не дает гарантий на автосериализацию в нужном разработчику формате так это и раньше так было.

То что коннектор станет давать лучшую автоконвертацию чем была факт. А кому надо те как раньше доконвертят вручную.

Важно что меняется обратная совместимость а значит исправление требует поднятие мажорной версии и отражения в BREAKING CHANGE к выпуску.

@alexandr-yang
Copy link
Contributor

Наткнулся на просторах гитхаба, связано с темой обсуждения
huxuxuya/KafkaConfluentRESTProxyAdapter1C#2

@leemuar
Copy link
Collaborator

leemuar commented Aug 6, 2024

Предлагаю компромисс: вводим отдельный метод и\или параметр, отвечающий за сериализацию.
Метод можно будет переопределить в расширении.
В параметре передавать форматную строку, которую применять к параметрам при их сериализации.

Таким образом мы сохраним текущее поведение и дадим возможность менять сериализацию параметров как каждому удобно.

ДопПараметры = Новый Структура("ФорматСериализацииПараметров", "ЧН=0; ЧГ=; ДФ=dd.MM.yy; БЛ=1; БИ=0");
КоннеткорHTTP.Post("https://ya.ru",, ДопПараметры);

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
5 participants