Skip to content

[QMS-1139] Fix: Endless loop with non-ASCII characters in VRT path(s)#1148

Open
kiozen wants to merge 2 commits into
Maproom:devfrom
kiozen:QMS--1139
Open

[QMS-1139] Fix: Endless loop with non-ASCII characters in VRT path(s)#1148
kiozen wants to merge 2 commits into
Maproom:devfrom
kiozen:QMS--1139

Conversation

@kiozen

@kiozen kiozen commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

GetFileList()/GetDescription() return filenames in the platform encoding (ANSI or UTF-8 on Windows). allReferencedFilesExist() decoded both ways, but buildOverviewAdvice() used QString::fromUtf8() unconditionally, so on ANSI Windows a non-ASCII path passed validation and then failed source probing/comparison. Centralize the decode in decodeGdalPath() and use it for the file list and ownPath; open probed sources trying UTF-8 (and local-8-bit as a fallback).

What is the linked issue for this pull request:

QMS-#1139

What you have done:

  • Added decodeGdalPath() helper and used it consistently for GetFileList()/GetDescription() decoding in CGdalVrtUtil.
  • Encoding-robust source open in probeSource().

Steps to perform a simple smoke test:

saee ticket

Does the code comply to the coding rules and naming conventions Coding Guidelines:

  • yes

Is every user facing string in a tr() macro?

  • yes

Did you add the ticket number and title into the changelog? Keep the numeric order in each release block.

  • yes, I didn't forget to change changelog.txt

@kiozen

kiozen commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

@JFritzle please test

@JFritzle

JFritzle commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Not fixed.

Last debug lines ...

2026-07-09 23:51:54.382 [debug] ------------------------------
2026-07-09 23:51:54.382 [debug] VRT: try to open "D:/Höhendaten/Höhendaten Kanaren.ANSI.vrt"

... before entering endless loop.

@kiozen

kiozen commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator Author

Ok, in that case I am out of ideas. This has to be fixed on the system it breaks.

@JFritzle

Copy link
Copy Markdown
Contributor
CGdalVrtUtil::overview_advice_t CGdalVrtUtil::buildOverviewAdvice(GDALDataset* dataset, GDALRasterBand* band,
                                                                  bool isPaletteIndexed,
                                                                  const QVector<qint32>& suggestedLevels) {
qInfo() << "buildOverviewAdvice 1";
  overview_advice_t result;
  result.suggestedLevels = suggestedLevels;
  result.isPaletteIndexed = isPaletteIndexed;
qInfo() << "buildOverviewAdvice 2";
  const qint32 targetFactor = suggestedLevels.isEmpty() ? 1 : suggestedLevels.last();
qInfo() << "buildOverviewAdvice 3";
  qDebug() << "OVR: GetOverviewCount =" << band->GetOverviewCount() << "targetFactor =" << targetFactor;
  ...

Last debug output: [info] buildOverviewAdvice 3
->band->GetOverviewCount() never ends.

@kiozen

kiozen commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator Author

Can you add a debug message right after GDALAllRegister() in IAppSetup

#include <cpl_conv.h>
....
GDALAllRegister();
  qDebug() << "GDAL_FILENAME_IS_UTF8 =" << CPLGetConfigOption("GDAL_FILENAME_IS_UTF8", "YES");
}

@JFritzle

Copy link
Copy Markdown
Contributor

Sorry, my fault.

When trying to check behavior of non-ASCII characters in VRT files, I have put German umlauts

  • into folder name
  • into VRT name
  • into filename referenced by VRT file
    The last one caused endless loop, as HGT files not following name convention [Letter][Latitude][Letter][Longitude] are invalid ...
gdalinfo "Höhe N27W016.hgt"
ERROR 4: `Höhe N27W016.hgt' not recognized as being in a supported file format.
gdalinfo "X27W016.hgt"
ERROR 4: `X27W016.hgt' not recognized as being in a supported file format.

... even if contents is the same as file N27W016.hgt.

@kiozen

kiozen commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator Author

Does that mean everything ok. Ready to merge?

@JFritzle

Copy link
Copy Markdown
Contributor

Please wait, I am still testing.

@JFritzle

Copy link
Copy Markdown
Contributor

There are still ANSI encoding problems:

File D:\Höhendaten\Höhendaten Schweiz.ANSI.vrt is an ANSI encoded file referencing relative file Höhendaten Schweiz/Höhendaten Schweiz.tif.

gdalinfo "D:/Höhendaten/Höhendaten Schweiz.ANSI.vrt" accepts file as valid an reports info,
even if gdalinfo output shows filelist as

Files: D:/H├Âhendaten/H├Âhendaten Schweiz.ANSI.vrt
       D:/H├Âhendaten/H÷hendaten Schweiz/H÷hendaten Schweiz.tif

Checking filelist for validity in QMS using method before this PR but armed with additional output, shows

[QMS] 2026-07-10 10:41:30.342 [debug] VRT: try to open "D:/Höhendaten/Höhendaten Schweiz.ANSI.vrt"
[QMS] 2026-07-10 10:41:30.452 [info] file 0= D:/Höhendaten/Höhendaten Schweiz.ANSI.vrt
[QMS] 2026-07-10 10:41:30.452 [info] 8bit= "D:/Höhendaten/Höhendaten Schweiz.ANSI.vrt"
[QMS] 2026-07-10 10:41:30.452 [info] utf8= "D:/Höhendaten/Höhendaten Schweiz.ANSI.vrt"
[QMS] 2026-07-10 10:41:30.452 [info] exists
[QMS] 2026-07-10 10:41:30.452 [info] file 1= D:/Höhendaten/H�hendaten Schweiz/H�hendaten Schweiz.tif
[QMS] 2026-07-10 10:41:30.452 [info] 8bit= "D:/Höhendaten/Höhendaten Schweiz/Höhendaten Schweiz.tif"
[QMS] 2026-07-10 10:41:30.452 [info] utf8= "D:/Höhendaten/H�hendaten Schweiz/H�hendaten Schweiz.tif"
[QMS] 2026-07-10 10:41:30.452 [info] does not exist

In contrast to gdalinfo, neither old method nor current PR's method recognizes VRT as valid.

@kiozen

kiozen commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator Author

Can you please fix it. I can't test and guessing fixes won't lead us to anywhere good.

@JFritzle

Copy link
Copy Markdown
Contributor

I will do my best.

@JFritzle

Copy link
Copy Markdown
Contributor

Skipping function CGdalVrtUtil::allReferencedFilesExist at all:

[QMS] 2026-07-10 12:39:30.314 [debug] VRT: try to open "D:/Höhendaten/Höhendaten Schweiz.ANSI.vrt"
[QMS] 2026-07-10 12:39:30.362 [debug] OVR: GetOverviewCount = 0 targetFactor = 16
[QMS] 2026-07-10 12:39:30.362 [debug] OVR: step 1: rawContainerSizes = QList() hasOwnOvr = false sufficient = false
[QMS] ERROR 4: D:/Höhendaten/H�hendaten Schweiz/H�hendaten Schweiz.tif: No such file or directory
[QMS] 2026-07-10 12:39:30.363 [debug] OVR:   "H�hendaten Schweiz.tif" checked: true sufficient: false sizes: QList()
[QMS] 2026-07-10 12:39:30.363 [debug] OVR: step 3: sawAnySource = true allSourcesHaveOverviews = false fallbackTrusted = false
[QMS] 2026-07-10 12:39:30.363 [debug] OVR: pre-warp result: containerSufficient = false containerHasOwnOvr = false subfileCount = 1 tooManySubfiles = false
[QMS] 2026-07-10 12:39:30.363 [debug] no data: 1 -32767

@kiozen

kiozen commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator Author

My guess is that it is a GDAL problem. The VRT driver has to handle the sub-files and their overview files or it's own overview file.

I would expect the GDAL API to be UTF8 only. But I can't tell if that holds true for Windows. A key might be the variable GDAL_FILENAME_IS_UTF8. From the docs:

GDAL_FILENAME_IS_UTF8=[YES​/​NO]: Defaults to YES. This option only has an effect on Windows systems (using cpl_vsil_win32.cpp). If set to "NO" then filenames passed to functions like VSIFOpenL() will be passed on directly to CreateFile() instead of being converted from UTF-8 to wchar_t and passed to CreateFileW(). This effectively restores the pre-GDAL1.8 behavior for handling filenames on Windows and might be appropriate for applications that treat filenames as being in the local encoding.

@JFritzle

Copy link
Copy Markdown
Contributor
gdalinfo --config GDAL_FILENAME_IS_UTF8 NO "D:/Höhendaten/Höhendaten Schweiz.ANSI.vrt"
ERROR 4: D:/H├Âhendaten/H├Âhendaten Schweiz.ANSI.vrt: No such file or directory
gdalinfo failed - unable to open 'D:/H├Âhendaten/H├Âhendaten Schweiz.ANSI.vrt'.

-> GDAL_FILENAME_IS_UTF8=NO makes it even worse.

The problem seems indeed to be a GDAL internal one:
An UTF-8 encoded VRT filename is passed to GDALOpen.
VRT file contents is ANSI encoded and references ANSI encoded relative filenames.
GetFileList returns absolute filenames, where UTF-8 encoded VRT file's folder name is concatenated with referenced ANSI encoded filenames. The final absolute filename is neither UTF-8 nor ANSI, neither QString::fromLocal8Bit nor QString::fromUtf8 returns the name of an existing file.

One possible solution could be:
Before passing VRT filename to GDALOpen, file is read and checked for UTF-8 contents.
If true, filename is passed to GDALOpen.
If false, contents is converted to UTF-8. Converted contents is written to a tempory file. Temporary filename is passed to GDALOpen.
Now returned filelist is reliable UTF-8 and file existence can get checked by QString::fromUtf8.

What do you think about?

One risk remains: if ANSI encoded VRT files are nested, only toplevel VRT is UTF-8 encoded. This does not matter, as long as referenced files are not checked recursively which currently does neither CGdalVrtUtil::allReferencedFilesExist nor CGdalVrtUtil::buildOverviewAdvice.

@JFritzle

Copy link
Copy Markdown
Contributor

Tested above. It is no solution, because referenced relative filenames are no longer relative to temporary file.

Other possible solution 1:

Before passing VRT filename to GDALOpen, file is read and checked for UTF-8 contents.
If true, filename is passed to GDALOpen.
If false, contents is converted to UTF-8. VRT file contents is silently replaced by UTF-8 converted contents.
If replacement fails due to permissions, a message box demands for manual conversion to UTF-8 and ANSI VRT file is rejected,
Now returned filelist is reliable UTF-8 and file existence can get checked by QString::fromUtf8.

Other possible solution 2:

Before passing VRT filename to GDALOpen, file is read and checked for UTF-8 contents.
If true, filename is passed to GDALOpen.
If false, a message box demands for manual conversion to UTF-8 and ANSI VRT file is rejected,

@kiozen

kiozen commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator Author

Sounds both like foot fungus or scabies.

As far as I understood, the key is to have a VRT that is 100% utf8, Right? Imho a case for the new VRT advisory dialog, File is checked and flagged with the exclamation mark badge if not 100% UTF8. User can click badge and gets informed about the defect with button to fix.

But really, it's 2026 and I never would have thought that this is still such a problem. Just because Microsoft can't get their shit fixed. UFT8 is today's standard and there is nor real reason why every software has to bitch around Microsoft's defect. In other words: My preferred fix is still switching Windows to UTF8 instead of adding peculiar features with quite a maintenance burden.

@kiozen

kiozen commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator Author

Another question: If the VRT is 100% UTF8, does it still need all the Lex Microsoft patches in this PR? I guess not, right?

@JFritzle

Copy link
Copy Markdown
Contributor

If VRTs are 100% UTF8, they do not need any special treatment nor patches from this PR.
In addition, section #if defined(Q_OS_WIN32) ... #endif can get removed from function CGdalVrtUtil::allReferencedFilesExist.

We had already discussions about Windows and UTF-8, at least in #1061 and #1067.
General UTF-8 compliance is still marked BETA und not enabled by default.
Enabling it might have impacts on executables not UTF-8 aware. And existing ANSI encoded text files ...
grafik
... may no longer be correctly encoded after switching to UTF-8 (i.e. code page 65001):
grafik

Until now I believed, general UTF-8 compliance can only be enabled as a system wide option. Reading Wiki I have learned, that since May 2019, it can be set per program. How to do is by compiler option /utf-8 described here.

I will give it a try, add option to CMakeLists.txt, re-build QMS, keep general UTF-8 switch off, and see what happens.
What I already know, ANSI encoded VRTs containing non-ASCII characters will no longer be recognized as valid.

@JFritzle

JFritzle commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

It turns out, that setting /utf-8 or not setting has no impact on QMS runtime behavior.
However adding section ...

  <application>
    <windowsSettings>
      <activeCodePage xmlns="http://schemas.microsoft.com/SMI/2019/WindowsSettings">UTF-8</activeCodePage>
    </windowsSettings>
  </application>

... to application executable's embedded manifest makes QMS behave same as if system wide UTF-8 had been enabled, even if system wide UTF-8 is not enabled.

Have to find out how to modify CMakeList.txt.

Having a manifest file codepage.UTF8.manifest, in order to merge code page UTF-8 settings, command
mt.exe -inputresource:qmapshack.exe;#1 -manifest codepage.UTF8.manifest -outputresource:qmapshack.exe;#1
has to be executed after successful building executable qmapshack.exe. While I succeed in running command manually, I did not find out how to integrate into CMakeLists.txt using cmake syntax and variables. Any help from cmake experts appreciated!!!!

@kiozen kiozen force-pushed the QMS--1139 branch 3 times, most recently from 4ed6622 to d14890f Compare July 12, 2026 09:42
@kiozen

kiozen commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator Author

Can you test the following with the latest patches:

  1. Manifest embeds and merges — mt.exe -inputresource:qmapshack.exe;#1 -out:check.manifest shows both the UTF-8 code page and the linker's usual entries (common-controls, execution level).
  2. A genuine UTF-8 VRT with umlauts in path/contents loads and renders (with the overview advisory).
  3. Routino still routes with a database in a non-ASCII path (e.g. D:/Börlin/...) — exercises the .toUtf8() change.

@wthaem

wthaem commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

This PR fails to compile in my case!

CMake Error at CMakeLists.txt:213 (if):
  if given arguments:

    "VERSION_LESS" "3.10.0"

  Unknown arguments specified

Should GDAL/PROJ_VERSION be set manually or should it be found automatically? Manually setting looks awkward! I could insert any suitable value to satisfy the if.

@wthaem

wthaem commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Please check if in CMakeLists.txt the above mentioned if statement is correct or if it should be

if("${PROJ_VERSION}" VERSION_LESS "9.4.0")

Copilot explanation:

The expression:

if(${PROJ_VERSION} VERSION_LESS 9.4.0)

does not compare the string "9.7.1" to "9.4.0".
Instead, it expands to:

if(9.7.1 VERSION_LESS 9.4.0)

…and CMake interprets 9.7.1 as a variable reference, not a literal version string.

Since no variable named 9.7.1 exists, CMake expands it to an empty string.

@kiozen

kiozen commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator Author

I rebased the branch. Should work now. If not @JFritzle has to explain. This is because of the new min version check.

@wthaem

wthaem commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Routino routing using data from directory Routing_äöüßложxxaáäçčяեω𝔊𝑃𝕆𝕏𝚽 was ok. Anything else not yet tested.

@JFritzle

Copy link
Copy Markdown
Contributor

QMS now always runs with UTF-8 encoding. Functions QString::fromLocal8Bit and QString::fromUtf8 behave identical on platform Windows too, as local encoding now is UTF-8 encoding.

Case distinction is no longer necessary and no longer possible. This was already the case when UTF-8 had been system-wide enabled.

As we are not able to guess the VRT file's encoding, I suggest to test if it is UTF-8 encoded, reject if not and throw an error message like that:
grafik
Here I have checked UTF-8 conformity in function CDemVRT::CDemVRT by

#if defined(Q_OS_WIN32)
  // Check for UTF-8 encoding
  QFile file(filename);
  if (!file.open(QIODevice::ReadOnly)) {
    QMessageBox::warning(CMainWindow::getBestWidgetForParent(), tr("Error..."),
                         tr("Failed to load file:") % '\n' % filename);
    return;
  }
  QByteArray byteArray = file.readAll();
  file.close();
  auto toUtf8 = QStringDecoder(QStringConverter::Utf8);
  QString string = toUtf8(byteArray);
  if (toUtf8.hasError()) {
    QMessageBox::warning(CMainWindow::getBestWidgetForParent(), tr("Error..."),
                       tr("File not UTF-8 encoded, conversion required:") % '\n' % filename);
    return;
  }
#endif  // defined(Q_OS_WIN32) 

This check could get encapsulated into a helper function and used where else required.

With QMapShack now running as a UTF-8 process on Windows (activeCodePage
manifest, QMS-1153), GDAL and Qt handle non-ASCII filenames natively. The
per-call encoding workarounds are therefore no longer needed and are
removed: the VRT path handling, the Routino path define, and the
qmt_map2jnx argv decoding.
@kiozen

kiozen commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator Author

@JFritzle please test. For now it's just the guard. I try to provide mitigation in ticket #1156

@JFritzle

JFritzle commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Two notes.

First one:
I get 2 message boxes which have to be closed in correct order.
grafik

Second one:
What happens in CSLDestroy(fileList); if value of fileList is nullptr?
Wouldn't it be safer to replace

bool CGdalVrtUtil::allReferencedFilesExist(GDALDataset* dataset, QString& missingFile) {
  char** fileList = dataset->GetFileList();
  bool allExist = true;
  for (qint32 n = 0; fileList != nullptr && fileList[n] != nullptr; ++n) {
    missingFile = QString::fromUtf8(fileList[n]);
  ...
  CSLDestroy(fileList);
  return allExist;
}

by

bool CGdalVrtUtil::allReferencedFilesExist(GDALDataset* dataset, QString& missingFile) {
  bool allExist = true;
  char** fileList = dataset->GetFileList();
  if (fileList == nullptr) {
    return allExist;
  }
  for (qint32 n = 0; fileList[n] != nullptr; ++n) {
    missingFile = QString::fromUtf8(fileList[n]);
  ...
  CSLDestroy(fileList);
  return allExist;
}

Beside of that, I agree to PR.

@kiozen

kiozen commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator Author

That multiple message box is only happening because there are multiple canvas. The file is activated on multiple canvas and stored in the config. Now, with the patch this triggers the message box multiple time. However if a message box is confirmed the file should not activate, this is saved in the config and next start should be ok. Is that right?

GDAL passes a VRT's <SourceFilename> bytes to the OS verbatim and ignores
the XML encoding declaration, so a non-UTF-8 (e.g. legacy Latin-1) VRT
resolves to broken paths and fails obscurely. Check UTF-8 validity when
opening a .vrt in CMapVRT/CDemVRT and reject it up front with a clear
message instead.

Also reconcile the map/DEM list indicator with the real activation outcome
so a rejected (or otherwise failed) file no longer leaves a stuck grey load
indicator in the tree.
@JFritzle

Copy link
Copy Markdown
Contributor

However if a message box is confirmed the file should not activate, this is saved in the config and next start should be ok. Is that right?

-> Not right. Same message boxes after restart,

Btw:
Some calls to toLocal8Bit/fromLocal8Bit are still remaining:

src/qmapshack/main.cpp:      argList += " \"" % QString::fromLocal8Bit(argVal[i]) % "\"";
src/qmaptool/main.cpp:      argList += " \"" % QString::fromLocal8Bit(argVal[i]) % "\"";
src/qmapshack/map/CMapGEMF.cpp:    source.name = QString().fromLocal8Bit(name, len);
src/qmapshack/gis/gpx/serialization.cpp:    isHtml = (attr.namedItem("html").nodeValue().toLocal8Bit().toLower() == "true");
src/qmapshack/gis/gpx/serialization.cpp:  geocache.archived = attr.namedItem("archived").nodeValue().toLocal8Bit().toLower() == "true";
src/qmapshack/gis/gpx/serialization.cpp:  geocache.available = attr.namedItem("available").nodeValue().toLocal8Bit().toLower() == "true";

@kiozen

kiozen commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator Author

-> Not right. Same message boxes after restart,

How? I need more information. No guess work. When activated the first time the message box pops up and activation is aborted. Closing QMS will save these VRTs as not active in the config. So a second restart should never activate them. If they are still active I need to know why.

This Windows bullshit ping pong is tiresome. I will never ever implement any Windows workaround in the future for sure. If it's broken on Windows it stays broken.

toLocal8Bit() should deflate to nothing on UTF8 systems. So it's more cosmetics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants