#prog #java #бомбёжкипост

В статье от PVS-Studio пишут про ошибку, найденную анализатором. Именно, в строке 8 исключение создаётся, но не выбрасывается. Я, признаться честно, это пропустил, но заметил (как и другой комментатор в оригинальном канале) другую ошибку: в строке 6 записывается индекс символа = в строке setting, которая является обрезанной settings[i], но в условии в строке 7 этот индекс проверяется в settings[i], изначальной, необрезанной строке. Намерение этого кода в данном блоке заключается в том, чтобы разбить строку на две половины до и после = и убедиться, что обе части непустые, однако из-за ошибки в условии проверка может пропустить пустую строку после =.

Я решил было написать код на Rust и продемонстрировать, насколько сложнее там допустить такую ошибку банально из-за более вменяемого API, но потом я решил глянуть на этот метод немного более широко. Именно, что этот код вообще делает? Он на каждый вызов разбирает строку от начала до конца, чтобы найти нужную подстроку. Он делает лишнюю работу, если вызывается больше хотя бы раза. Разумно было бы сделать этот разбор только один раз и сложить результаты в словарь. Это убрало бы оверхед разбора и заодно переместило бы валидацию значений с каждого вызова метода на единственное создание мапы.

Я решил посмотреть, как этот метод используется. И после этого у меня начало бомбить.

Так как это приватный метод, все его использования должны быть в этом же файле. И действительно, этот метод вызывается два раза, в package-private геттере getStorageEndpoint. И знаете, с чем он вызывается? С константами BLOB_ENDPOINT_NAME и BLOB_SECONDARY_ENDPOINT_NAME, определёнными несколькими строками выше, как "BlobEndpoint" и "BlobSecondaryEndpoint"! Окей, два вызова — это не настолько плохо (при условии, что сам getStorageEndpoint вызывается один раз), но, вероятно, строка для разбора передаётся извне, поэтому сам разбор имеет смысл?

А вот и нет! Именно, строка, разбираемая в getProperty — это результат вызова getConnectionString, тривиального геттера, возвращающего поле connectString. Значение этого поля, ожидаемо, выставляется в конструкторе (типа AzureStorageSettings), через вызов статического метода buildConnectString. Этот метод составляет строку вида key=value;key=value;... из переданных аргументов, и в число этих аргументов входят endpoint (который записывается после BlobEndpoint=) и secondaryEndpoint (который записывается после BlobSecondaryEndpoint=). Знакомые строки? Да, это именно те константы, которые используются для лукапа в getStorageEndpoint. Иными словами, в getStorageEndpoint вызывается код, который разбирает строку, чтобы получить оттуда значения, которые были переданы в конструктор типа напрямую! Просто сохранив значения аргументов, можно было бы выкинуть весь метод getProperty целиком и заменить его вызовы на два доступа к полям.

Но и это ещё не всё. В методе getStorageEndpoint возвращается запись StorageEndpoint, которая является просто парой строк. Даже имена полей дают понять, что эти два значения — URI. Хранятся ли они в виде URI (этот тип даже импортируется в начале файла)? Нет, они хранятся в типе String. Как тогда код валидирует, что это действительно URI? Никак... Точнее, не всегда. Именно, StorageEndpoint создаётся в двух местах в этом файле, как раз в методе getStorageEndpoint. В первом случае строки передаются напрямую, а во втором передаются результаты вызова метода deriveURIFromSettings. Этот метод конструирует строку из полей AzureStorageSettings, а в конце там вот такой восхитительный код:

        try {
            return new URI(uriString).toString();  // validates the URI
        } catch (URISyntaxException e) {
            throw new IllegalArgumentException(e);
        }

URI создаётся только для того, чтобы немедленно перевести его в строку и потерять гарантии валидности!

P. S.: посылаю лучи ненависти разработчикам нового UI GitHub, где визуальное отображения выделения текста не совпадает с реально выделенным текстом, а ссылки на результаты поиска символа не всегда переходят к нужной строке.