Изучаем карты с исходным кодом GeoServer18.09.2024 12:30

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

Geo вернулся

Уж не знаю, как так получилось, что сразу после написания статьи про GeoGebra я наткнулся на GeoServer. Видимо, этот префикс оставлять меня пока не хочет. Однако в отличие от предыдущего проекта семейства Geo (эти проекты никак не связаны) здесь речь пойдёт не о Geoметрии, а о Geoграфии.

Итак, что же такое GeoServer? Это сервер! Сервер, конечно, не простой, а такой, что обеспечивает данные и карты для клиентов вроде веб-браузеров или геоинформационных систем. Люди заинтересованные могут прочитать подробное описание и все его возможности на сайтах GeoServer, OSGeo или GeoSolutions. Но для того, чтобы понять, с чем имеем дело, предлагаю посмотреть интерфейс панели доступа сервера:

Уже на первом запуске предоставляются некоторые стартовые данные, так что можно даже визуально оценить их на карте:

Мы получаем в формате OpenLayers интерактивную карту мира и, кликнув по какому-либо месту, можем получить подробности.

Возможности сервера обрисовали. Будь время, я бы поигрался и попробовал на открытых данных (например, OpenStreetMap) собрать какую-нибудь интересную карту локального места, но пока остановлюсь на этом.

А теперь предлагаю заглянуть внутрь такого великолепного сервиса. И не просто посмотреть, а попробовать отыскать ошибки с использованием статического анализатора PVS-Studio. Разворачиваем карты кода и начинаем путешествие.

Опечатались

Две модели как одна

private GridSampleDimension[] getCoverageSampleDimensions(
    GridCoverage2DReader reader, Map customParameters)
    throws TransformException, IOException, Exception {
  ....
  ColorModel cm = imageLayout.getColorModel(null);
  if (cm == null) {
    throw new Exception(
        "Unable to acquire test coverage and color model for format:"
            + format.getName());
  }
  SampleModel sm = imageLayout.getSampleModel(null);
  if (cm == null) {
    throw new Exception(
        "Unable to acquire test coverage and sample model for format:"
            + format.getName());
  }
  ....
}

Создав две крайне похожие переменные cm и sm, легко перепутать их в будущем, а также не заметить проблемы на code review. Нетрудно понять, что здесь изначально планировали проверить sm на null, а не проверять уже проверенное. Исправляем опечатку и двигаемся дальше. На ошибку здесь указывают аж два срабатывания:

• V6007 Expression 'cm == null' is always false. CatalogBuilder.java 1242

• V6080 Consider checking for misprints. It’s possible that an assigned variable should be checked in the next condition. CatalogBuilder.java 1241, CatalogBuilder.java 1242

Необходимость нужной версии

public StyledLayerDescriptor run(final GetStylesRequest request)
 throws ServiceException {
  if (request.getSldVer() != null
      && "".equals(request.getSldVer())
      && !"1.0.0".equals(request.getSldVer()))
    throw new ServiceException("SLD version " + request.getSldVer() +
                               " not supported");
  ....
}

Здесь мы наблюдаем необычную проверку. Проверять *request.getSldVer () *одновременно на эквивалентность пустой строке и неэквивалентность 1.0.0 не имеет смысла, о чём и предупреждает анализатор:

V6057 Consider inspecting this expression. The expression is excessive or contains a misprint. GetStyles.java 40

Что же здесь произошло, и что должно быть?

Сообщение, которое мы должны получить в случае успешной проверки, сообщает нам о неподдерживаемой версии SLD. Я решил посмотреть, что есть SLD, и какие версии поддерживает GeoServer. На то и существует документация. Прямо из неё узнаём:

The OGC Styled Layer Descriptor (SLD) standard defines a language for expressing styling of geospatial data. GeoServer uses SLD as its primary styling language.

Вдаваться в подробности нет необходимости, но всё же неплохо бы уточнить что-то про версии этого стандарта. Дело нетрудное, смотрим немного ниже и находим:

GeoServer implements the SLD 1.0.0 standard, as well as some parts of the SE 1.1.0 and WMS-SLD 1.1.0 standards.

Понимаем, что поддерживаются версии 1.0.0 и 1.1.0. В нашем фрагменте кода мы действительно находим проверку на несовпадение версии 1.0.0, только вот необходимость эквивалентности пустой строке её ломает. Если в запросе будет нужная нам версия, то всё прекрасно. Но даже если версия будет, скажем, 3.5.2, то исключение всё равно не получим.

Я вижу два варианта: либо проверку на пустую строку необходимо удалить в принципе, либо на её месте должна быть проверка !»1.1.0».equals (request.getSldVersion ()), поскольку, согласно документации, эта версия стандарта частично поддерживается.

Высокий и широкий

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

int width = w instanceof Integer ? ((Integer)w) : Integer.parseInt((String)w);
int height = w instanceof Integer ? ((Integer)h) : Integer.parseInt((String)h);

И, собственно, сообщение:

V6072 Two similar code fragments were found. Perhaps, this is a typo and 'h' variable should be used instead of 'w'. Wcs10GetCoverageRequestReader.java 229, Wcs10GetCoverageRequestReader.java 229, Wcs10GetCoverageRequestReader.java 230, Wcs10GetCoverageRequestReader.java 230

Здесь для каких-либо пояснений можно заняться работой переводчика, но, думаю, все справятся и так.

Не сортировать несортируемое

@Override
public  CloseableIterator list(
    final Class of,
    final Filter filter,
    @Nullable Integer offset,
    @Nullable Integer count,
    @Nullable SortBy... sortOrder) {

  if (sortOrder != null) {                                // <=
    for (SortBy so : sortOrder) {
      if (sortOrder != null &&                            // <=
          !canSort(of, so.getPropertyName().getPropertyName())) {
        throw new IllegalArgumentException(
            "Can't sort objects of type "
                + of.getName()
                + " by "
                + so.getPropertyName());
      }
    }
  }
  ....
}

V6007 Expression 'sortOrder!= null' is always true. DefaultCatalogFacade.java 1138

Одна и та же проверка происходит дважды, причём никаких изменений по пути от одной к другой не происходит. Очевидно, что значение второй всегда будет true. Возможно, она просто лишняя?

Подозреваю, что проверить на null хотели элементы sortOrder, следовательно, должно быть so!= null. Ведь такая проверка защитила бы от потенциального NullPointerException при использовании so.getPropertyName ().

Избегая NullPointerException

Нужное ли проверили

protected void updateAttributeStats(DataAttribute attribute) 
    throws IOException {
  ....
  // check we can compute min and max
  PropertyDescriptor pd = fs.getSchema().getDescriptor(attribute.getName());
  Class binding = pd.getType().getBinding();
  if (pd == null
      || !Comparable.class.isAssignableFrom(binding)
      || Geometry.class.isAssignableFrom(binding)) {
    return;
  }
  ....
}

Что здесь подозрительного? Проверка pd == null. Предлагаю посмотреть предупреждение анализатора PVS-Studio и начать расследование:

V6060 The 'pd' reference was utilized before it was verified against null. DataPanel.java 117, DataPanel.java 118

Согласитесь, странно проверять переменную pd уже после её утилизации в виде pd.getType (). Будь она null, наши руки уже были бы заняты пойманным NullPointerException. Здесь у меня возникла некоторая гипотеза.

Предлагаю посмотреть на метод isAssignableFrom. Залезем в Javadocs, но я не стану приводить его целиком, ибо нас интересует лишь конкретная часть:

@throws NullPointerException if the specified Class parameter is null

Итак, у нас есть первая зацепка: binding не должен быть null. Далее предлагаю посмотреть, может ли pd.getType ().getBinding () вернуть null. Сам метод отсылает нас к интерфейсу PropertyType. Здесь мы уже находимся в зависимости, которая называется geotools. И, к счастью, вновь на помощь приходят Javadocs. Читаем про getBindings и получаем:

This value is never null.

Что ж, от своей гипотезы отказываюсь. Какая была гипотеза? Что проверить хотели binding.

А вот нужна ли проверка у pd? fs.getSchema () в нашем случае возвращает ComplexType. Находим getDescriptor, вновь читаем и узнаём:

This method returns null if no such property is found.

Прекрасно. В получении какого-либо значения в pd действительно необходимо убедиться, однако эта проверка осуществляется слишком поздно.

Где мой SpringSecurityException?

@Test
public void testChallenge() throws Exception {
  Map raw = getWorld();
  try {
    executeGetCoverageKvp(raw);
    fail("This should have failed with a security exception");
  } catch (Throwable e) {
    // make sure we are dealing with some security exception
    Throwable se = null;
    while (e.getCause() != null && e.getCause() != e) {
      e = e.getCause();
      if (SecurityUtils.isSecurityException(e)) {
        se = e;
      }
    }
  
    if (e == null) {                                              // <=
      fail("We should have got some sort of SpringSecurityException");
    } else {
      // some mumbling about not having enough privileges
      assertTrue(se.getMessage().contains("World"));
      assertTrue(se.getMessage().contains("privileges"));
    }
  }
}

Обнаруживаем себя в тесте. Здесь происходит проверка на получение SpringSecurityException. Однако PVS-Studio находит аномалию в проверке e == null:

V6060 The 'e' reference was utilized before it was verified against null. ResourceAccessManagerWCSTest.java 186, ResourceAccessManagerWCSTest.java 193

Действительно, зачем переменную проверять на null, если её уже использовали? К великому счастью, код здесь обладает хорошей концентрацией комментариев и пояснительным сообщением в fail. Ожидается, что мы получим исключение безопасности, а оно записывается в переменную se. Эту самую переменную на null и необходимо проверить. Кстати, поскольку e == null никогда не вернёт true, то этот тест спокойно проходит.

Ранняя утилизация

public CoverageViewAbstractPage(
    String workspaceName, String storeName, 
    String coverageName, CoverageInfo coverageInfo)
    throws IOException {
  ....
  // grab the coverage view
  coverageViewInfo =
      coverageInfo != null
          ? coverageInfo
          : catalog.getResourceByStore(store, coverageName, CoverageInfo.class);
  CoverageView coverageView =
      coverageViewInfo
          .getMetadata()                                                   // <=
          .get(CoverageView.COVERAGE_VIEW, CoverageView.class);
  // the type can be still not saved
  if (coverageViewInfo != null) {                                          // <=
    coverageInfoId = coverageViewInfo.getId();
  }
  if (coverageView == null) {
    throw new IllegalArgumentException(
        "The specified coverage does not have a coverage view attached to it");
  }
  ....
}

V6060 The 'coverageViewInfo' reference was utilized before it was verified against null. CoverageViewAbstractPage.java 128, CoverageViewAbstractPage.java 132

Встретили ещё одну сомнительную проверку. Наличие здесь проверки на null вызывает вопросы. Если ожидается, что coverageViewInfo может быть null, то coverageViewInfo.getMetadata () может выбросить NullPointerException. Не похоже на опечатку, так что необходимо попробовать разобраться, что происходит. Начнём с обращения к git blame.

Весь код, что находится между двумя комментариями, относится к одному коммиту, остальное — к другому. Всего в истории обнаружены два коммита. Оригинальный выглядит так:

public CoverageViewAbstractPage(
    String workspaceName, String storeName, 
    String coverageName, CoverageInfo coverageInfo)
    throws IOException {
  ....
  // grab the coverage view
  coverageViewInfo =
      coverageInfo != null ? coverageInfo : catalog.getResourceByStore(
      store, coverageName, CoverageInfo.class);
  CoverageView coverageView = coverageViewInfo.getMetadata().get(
      CoverageView.COVERAGE_VIEW, CoverageView.class);
  // the type can be still not saved
  if (coverageViewInfo != null) {
    coverageInfoId = coverageViewInfo.getId();
  }
  if (coverageView == null) {
    throw new IllegalArgumentException(
        "The specified coverage does not have a coverage view attached to it");
  }
  ....
}

В логике ничего не изменилось — проверка проводится после использования, так что потери логики из-за слияния различных коммитов не произошло.

Передвижение блока с проверкой выше не имеет смысла: логика останется прежней и coverageViewInfo.getMetadata () исполнится, даже если coverageViewInfo == null. Другой вариант — банально бросать IllegalArgumentException, если coverageViewInfo == null (подобно проверке coverageView == null). Выходит, мы просто заменим одно исключение другим? Возможно. Однако теперь к нему можно добавить пояснительное сообщение.

Забытые не вернулись

Верните мою коллекцию

Вашему вниманию представляются сразу три метода, содержащие одну и ту же проблему:

public TreeSet