Давайте я сразу зайду с козырей. Сколько ошибок в коде этой функции вы можете найти за 60 секунд?


func NewConnectionString(host, path, database, user, password string, debug bool) string {
	return fmt.Sprintf(
		"proto://%s/%s?userName=%s&password=%s&database=%s&debug=%t",
		host, path, database, user, password, debug,
	)
}

Все ошибки в этом довольно небольшом коде найти и обезвредить довольно сложно. Я попробую их сейчас сформулировать и скомпоновать в две основные:


  • очевидная — перепутаны параметры;
  • не очевидная — параметры не экранируются.

Ладно, признавайтесь — наверняка каждый за 60 секунд смог заметить первую ошибку, но не все из вас смогут заметить вторую. Если ваш список ошибок оказался короче, чем мой — тогда прошу под кат.



История проблемы


Не подумайте, что тема высосана из пальца. Хоть и не часто, но она порой всплывает то тут, то там. То при клиент-серверном взаимодействии, когда разработчики не использовали openapi кодогенерацию и писали код вручную. То при генерации ConnectionString для драйвера базы данных. Да мало ли где можно ошибиться с этим дурацким URL?


Какие последствия? Да очень разнообразные, выбирайте на свой вкус:


логические ошибки


В некотором кол-ве случаев, когда параметр содержит недопустимый для query символ, весь URL либо становится полностью невалидным, либо мы получаем искаженный, но рабочий запрос. Если в первом случае мы можем заметить записи в журнале ошибок, то второй случай гораздо хуже — мы не сможем вовремя заметить ошибку. Возможно, даже никогда.


ошибки при внесении изменений


Не поверите, но есть такая история. В какой-то момент, когда разработчик изменял такую функцию, он перепутал местоположение плейсхолдера и параметра. И в такой форме функция существовала довольно долгое время. Почему никто не заметил проблемы? Да потому, что имя пользователя, пароль и наименование базы данных совпадало. А в тот момент, когда в этой схеме обновили уровень безопасности, и пароли стали генерироваться случайным порядком, все сломалось. Понятия не имею, сколько разработчик потратил времени на обнаружение ошибки, но когда я увидел эту функцию, я не заметил ошибки вообще.


нет гарантии корректности на этапе компиляции


Это очень важный пункт. Как мы все знаем, ошибки бывают двух основных видов: run-time и compile-time. Т.е. ошибки времени выполнения и времени компиляции соответственно. Какой вид хуже? Конечно ошибки времени выполнения. Почему? Да потому, что программа уже выполняется, а это значит, что данные уже могли пострадать. Кроме того, такие ошибки нужно сначала обнаружить и локализовать. Если мы вдруг пропустим знак амперсанда между параметрами — никто нам этого не скажет.


сложности экранирования


Поверьте мне на слово — экранировать правильно все части URL строки не так-то просто. Там есть, по крайней мере, три части, правила экранирования символов в которых отличаются. И я знаю не один пример реальных процессов коммерческой разработки, когда в связи с этим было принято решение ограничить допустимые символы для логина и пароля.


query injection


Да. Это почти как SQL injection, только query. Ну да, использование этой уязвимости имеет довольно узкий круг возможностей. Фактически, почти никаких возможностей, если мы не даем пользователю возможность влиять на параметры. Но все же, если отмести всякие контексты, и взглянуть правде в лицо, то это самая настоящая уязвимость.



Вона уже, как много смог описать. Надеюсь, я убедил вас в существовании реальной проблемы. Тогда давайте с ней бороться.



А как правильно?


Пакет net/url является стандартным пакетом языка GO и предоставляет довольно простой и удобный инструмент для формирования URL со всеми прилагающимися плюшками (согласно RFC 3986): гарантия синтаксической правильности, экранирование по всем канонам и проверка литералов на этапе компиляции.


Давайте еще раз посмотрим на “неправильный” код, я немного усложнил его расположением логина и пароля перед именем сервера, как положено по старому стандарту:


func NewSprintfConnectionString(proto, host, path, database, user, password string, debug bool) string {
	return fmt.Sprintf("%s://%s:%s@%s%s?database=%s&debug=%t", proto, user, password, host, path, database, debug)
}

А теперь код, который структурирован:
func NewURLConnectionString(proto, host, path, database, user, password string, debug bool) string {
	const (
		cDataBaseURLParameter = "database"
		cDebugURLParameter    = "debug"
	)
	var v = make(url.Values)
	v.Set(cDataBaseURLParameter, database)
	if debug {
		v.Set(cDebugURLParameter, "true")
	} else {
		v.Set(cDebugURLParameter, "false")
	}
	var u = url.URL{
		Scheme:   proto,
		Host:     host,
		Path:     path,
		User:     url.UserPassword(user, password),
		RawQuery: v.Encode(),
	}
	return u.String()
}

Как вы могли заметить, код заметно увеличился. Но стал ли он сложнее? Давайте разберемся.
В верхней части я объявил константы, которые отвечают за наименование параметров. Это не обязательно и кроме того, если вы реализуете какой-то межпроцессный обмен между двумя сервисами вашей системы, возможно, эти константы уже есть. Я их выделил для того, чтобы не было magic-words. Это позволяет видеть мне список всех возможных параметров в одном месте и не дает мне ошибиться при многократном использовании одного и того же имени параметра в коде программы в нескольких местах. Я имею в виду использование cDebugURLParameter дважды — в случае, если мы используем magic-words у нас каждый раз есть риск допустить опечатку.


Следом я создаю и затем наполняю url.Values — структура, которая поможет мне собрать разные параметры query и затем собрать из них экранированную query строку. Обращаю внимание на то, что в такой форме код выходит довольно наглядным — мы видим имя параметра и прямо напротив него значение. Сравните расположение имен и значений в варианте с использованием fmt.Sprintf.


В нижней части литерал url.URL в котором я заполняю необходимые элементы URL строки: протокол, хост, путь и прочее. Обратите внимание на то, как используются параметры user и password. Любые символы в имени пользователя и пароле будут корректно экранированы, мы можем не ограничиваться в каких-то диапазонах, которые потом еще придется контролировать.


Обозримость кода существенно не изменилась — кол-во строк меньше 30, зато по ширине код стал компактнее и приобрел наглядность. Использование литерала url.URL и структуры url.Values позволяет заручиться гарантиями компилятора, что я не нарушу правил расположения амперсанд и других символов. Специальные символы, которые были внесены аргументами функции будут гарантировано экранированы по всем канонам (как я уже и говорил).


Ну и как вишенка на торте — сложность внесения изменений в эту функцию не зависит от количества параметров. Т.е. остается константой. Представьте, что у нас есть 7 параметров, а нам нужно убрать один из них, а затем добавить еще 7. В случае с форматной строкой (fmt.Sprintf), нам нужно быть аккуратными при внесении изменений, а если мы используем net/url, нам нужно добавить константы, которые будут нести имена новых параметров, а затем заполнить параметры с помощью v.Set(cDebugURLParameter, "false"). Что тут может пойти не так?


А что там с производительностью?


Ну да, придется немного добавить горчинки в мой салат. За все нужно платить, и нам придется заплатить за гарантии безопасности нашей URL строки производительностью. Сколько стоит? Давайте проверим. Для начала соберем вот такой бенчмарк:


Посмотреть код бенчмарка
func benchConnectionStringFunction(b *testing.B, fn func(proto, host, path, database, user, password string, debug bool) string) {
	for i := 0; i < b.N; i++ {
		_ = fn(
			"http",
			"my-host-name.com",
			"/users/preferences/email",
			"storage",
			"vladimir@yandex.ru",
			"foo-bar$100",
			false,
		)
	}
}

func BenchmarkNewSprintfConnectionString(b *testing.B) {
	benchConnectionStringFunction(b, NewSprintfConnectionString)
}

func BenchmarkNewURLConnectionString(b *testing.B) {
	benchConnectionStringFunction(b, NewURLConnectionString)
}

func BenchmarkNewConcatenateConnectionString(b *testing.B) {
	benchConnectionStringFunction(b, NewConcatenateConnectionString)
}


Тут у нас
  • NewSprintfConnectionString — это вариант с fmt.Sprintf;
  • NewURLConnectionString — это вариант с net/url;
  • NewConcatenateConnectionString — это секретный вариант, который я раскрою позже, правда по названию уже можно догадаться в чем секрет.

Принимаем за номинальную производительность первый вариант. Ну так как я его критикую, как довольно простое и довольно неверное решение, считаю, что производительность нужно оценивать относительно этого варианта.


Итак, что там у нас получается?
Время: 435 ns/op
Память: 208 B/op
Аллокаций: 7 allocs/op

Довольно производительный вариант. Немного расстраивает количеством аллокаций, но тут ожидаемо, ведь все параметры для fmt.Sprintf приходят, как interface{} — а это всем известная драма.


Окей. Следующий на разделочной доске вариант с net/url (мой любимый). Сразу скажу, все map — это в любом случае аллокация в памяти, кроме того, если слайс используется вне функции (а в нашем случае у нас url.Values это как раз карта слайсов: map[string][]string), то он тоже приводит к аллокации. Ну и сложные манипуляции со строками внутри реализации url.URL.String() тоже не без аллокаций. Так что (барабанная дробь):


Время: 1237 ns/op
Память: 608 B/op
Аллокаций: 14 allocs/op

Ну на самом деле совсем неплохо, учитывая то, что все переменные (включая аргументы функции) утекли в heap. Мы получили всего троекратное проседание по производительности. Полторы микросекунды на операцию я считаю не такой уж большой платой за качество кода и стабильность процесса разработки. И если бы потребовалось немного затюнить производительность, то я бы обратил внимание на эту функцию, при условии многократного ее выполнения. Но пришел бы к очевидному выводу — проблема производительности при общении с сетью кроется совсем не во времени формирования URL, а на переходах в уровни ядра, времени передачи данных по сети и все связанные с этим накладные расходы. Так что к чему вообще все эти бенчмарки?


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


Время: 416 ns/op
Память: 160 B/op
Аллокаций: 3 allocs/op

Довольно неплохо. Тут радует кол-во аллокаций. Да и по памяти есть существенный выигрыш по сравнению с первой функцией. Ну давайте глянем, что там за секрет.


Обратно в каменный век


Все знают, что один из самых производительных способов собрать строку в GO — это простая операция конкатенации. И вот функция построенная на этом простом принципе:


func NewConcatenateConnectionString(proto, host, path, database, user, password string, debug bool) string {
	var appendix = "&debug=false"
	if debug {
		appendix = "&debug=true"
	}
	return proto + "://" + url.QueryEscape(user) + ":" + url.QueryEscape(password) + "@" + host + path + "?database=" + url.QueryEscape(database) + appendix
}

Ну да, я зачем-то отступил от своего принципа и отправился обратно в каменный век: потерял возможность экранировать path, допустил некоторые деградации в экранировании user и password (там есть разница экранирования с query). В целом выглядит не очень.


Но давайте взглянем правде в лицо. Допустить ошибку при изменении такой функции все-таки сложнее, чем в функции с использованием fmt.Sprintf не смотря на то, что код этой функции выглядит довольно печально.


Давайте попробуем избавиться от уродства этой функции, добавим в нее немного красоты.


func NewConcatenateConnectionString(proto, host, path, database, user, password string, debug bool) string {
	var URL strings.Builder
	URL.Grow(128)
	URL.WriteString(proto + "://")
	if user != "" {
		URL.WriteString(url.QueryEscape(user) + ":" + url.QueryEscape(password) + "@")
	}
	URL.WriteString(host + path)
	URL.WriteString("?database=" + url.QueryEscape(database))
	if debug {
		URL.WriteString("&debug=true")
	} else {
		URL.WriteString("&debug=false")
	}
	return URL.String()
}

Удалось вернуть немного наглядности и структурированности: мы снова видим параметр напротив значения: URL.WriteString("?database=" + url.QueryEscape(database)) за сравнительно небольшую плату. Давайте глянем, какую.


Время: 478 ns/op
Память: 272 B/op
Аллокации: 5 allocs/op

Мне кажется, что этот вариант будет слегка поприличнее, чем вариант с fmt.Sprintf. Но давайте разберем, какие у него есть преимущества и недостатки (в сравнении с эталонным).


1 ый минус: Нет возможности обозреть шаблон итогового URL — я имею в виду, что по сравнению с форматной строкой, которая представляет нам хоть и не очень наглядно, но зато целиком весь URL, который мы можем получить, этот случай имеет просто набор некоторых этапов заполнения строки, из которой в итоге возникнет URL. Довольно слабый минус, вряд ли кому-то захочется видеть пример формируемого URL, а если захочется, его можно вложить в комментарии.


2 ой минус: Все параметры необходимо привести к типу данных строка — обратили внимание на то, как пришлось поступить с параметром debug? А представьте, что придется сделать с целочисленными параметрами! Но в любом случае, тут никто не запрещает использовать fmt.Sprintf внутри вызова URL.WriteString. Так что этот минус еще слабее предыдущего.


1 ый плюс: Более гибкая логика. В варианте с простым fmt.Sprintf мы можем транслировать параметры в URL строку как они есть, а это значит, что готовить их нужно до передачи в функцию. В нашем же случае мы можем дополнять функцию какой-то логикой и даже опускать некоторые параметры, как это сделано с user+password.


2 ой плюс: Стоимость доработок. С увеличением кол-ва параметров сложность не изменяется — мы не будем увеличивать длину форматной строки и все большее время тратить на внимание к правильности очередности имен параметров и их значений. А вот это, на мой взгляд, довольно существенный плюс.


Победитель


Ну и раз это номинация, давайте же выберем победителя через нечестное и предвзятое судейство. Оценивать буду по следующим критериям (они соотносятся с проблемами, которые описаны выше и добавим еще производительность):


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


fmt.Sprintf net/url strings.Builder
логика 1 3 2
сложность изменения 1 3 2
гарантии компилятора 1 3 2
производительность 2 1 3
Итог 5 10 9

Думаю, что победитель в номинации “худший способ сформировать URL строку в golang” нами обнаружен. Прошу вас, по возможности не используйте этот способ.



Комментарии (9)


  1. danilovmy
    28.03.2022 13:57
    +4

    Логин и Пароль в querystring? Или мои глаза меня подводят?


    1. devalio Автор
      28.03.2022 14:10
      +3

      Ну, это имеет значение только, если процесс происходит вне вашей инфраструктуры. Т.е. браузерная адресная строка и подобное
      Все URL подключения к базам данных и любым другим сервисам внутри вашей инфраструктуры это нормально переносят. А те, что вне - подключаемся через TLS и снова все ок
      Т.е., если грубо, то скажу так: на бэкэндщиков это правило не действует =)

      Ведь фактически всем безразлично, где вы передаете логин и пароль - самое главное, чтобы это было защищено. А если это в адресной строке браузера - то это уже видно на экране (полагаю, что еще и HTTP прокси увидит, если он есть) - т.е. не секюрно

      Короче, не угадали немного


  1. Vest
    28.03.2022 18:24

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


    1. devalio Автор
      28.03.2022 18:29

      Не совсем понимаю, где тут "привирание" - Grow(128) - тут происходит выделение буфера памяти - да, это аллокация, а возможно, если не хватит этого буфера, то будет еще одна аллокация

      Кешировать параметры никто не собирается - тема статьи не про то, как производительнее конкатенировать строки. А про то, что Sprintf - это плохой способ собрать URL

      fmt.Sprintf - это плохой способ собрать URL!


  1. Tatikoma
    28.03.2022 18:36

    Перепутанные параметры не заметил, но резануло глаз то что для аргумента password указан тип данных, а для остальных - не указан. Конкретно на Go не программирую, - там не будет связанных с этим проблем? - например если в host передать объект, массив, число, булев, нулл?


    1. devalio Автор
      28.03.2022 18:39

      Нет, у нас все просто. Через запятую однотипные параметры, через пробел тип - это Ок

      Передать случайно туда другой тип (булев или нулл) не получится - строгая типизация. Компилятор не даст


    1. GoodGod
      28.03.2022 18:40

      Когда ряд последовательно идущих параметров функции имеют одинаковый тип, то тип для них всех можно указывать 1 раз после последнего параметра.


  1. 4lex
    29.03.2022 09:27
    +3

    А что мешает Вам ошибиться в порядке аргументов NewURLConnectionString?


    1. devalio Автор
      29.03.2022 10:41

      Порядок следования параметров не важен, если значения связаны с параметрами. А вот если наименования параметров и передаваемые значения не имеют сцепки, тогда порядок становится важен. Только давайте я назову это "порядок передаваемых значений". Вот поглядите:

      // мне не важно, вот так:
      v.Set("server_name", serverName)
      v.Set("database_name", databaseName)
      // или вот так:
      v.Set("database_name", databaseName)
      v.Set("server_name", serverName)

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

      // мне важно вот так
      fmt.Sprintf("?database_name=%s&server_name=%s", serverName, databaseName)

      А теперь этот пример с двумя параметрами увеличьте мысленно хотя бы до шести параметров, да еще прикольно будет, если в имени каждого будет _name.

      Так что - глаза. Мой ответ "глаза мне мешают ошибиться в порядке передаваемых значений".