TwinFan / LiveTraffic

LiveTraffic is an X-Plane multiplayer plugin, which fills your sky with live air traffic based on public flight tracking data.
https://twinfan.gitbook.io/livetraffic/
Other
101 stars 24 forks source link

"incorrect" locale setting causes malformed URL generation #244

Closed sfrsfrsfr closed 1 year ago

sfrsfrsfr commented 2 years ago

hello,

i was getting these errors when setting up livetraffic with an opensky account:

LiveTraffic WARN LTChannel.cpp:610/FetchAllData: OpenSky Live Online: HTTP response is not OK but 400 for https://opensky-network.org/api/states/all?lamin=49,808&lomin=8,188&lamax=50,267&lomax=8,901

Notice the , used in the coordinates

Expected behavior

No such error :-)

My Current locale setting: LC_CTYPE="en_US.UTF-8" LC_NUMERIC=de_DE.UTF-8

After changing LC_NUMERIC to export LC_NUMERIC="en_US.UTF-8" everything worked as expected

Technical Info

Suggestion: the LC_NUMERIC locale setting should not affect the URL generation used to fetch traffic data.

Thanks for a wonderful plugin!

TwinFan commented 2 years ago

Same as here in the forum but with actually useful info included, so might be able to work on it.

TwinFan commented 2 years ago

setlocale() would set the locale globally, not even thread-safe, so it would affect anything running in any parallel X-Plane thread, potentially mid-sprintf… That‘s way too dangerous.

So the solution will be to query the locale, and if the decimal_point is different from . then it would be replaced afterwards in the url string.

TwinFan commented 2 years ago

@sfrsfrsfr , can you try the attached version to see if that fixes your problem. I am pretty sure that the correct URL gets produced. I'm just a bit concerned that down the road you might end up with JSON parsing errors as that is what happened during my testing. Not totally unreasonable...the locale will influence interpreting strings as lists and numbers, too. But I'm not on Linux and the way I overruled the locale might not have been proper.

Feedback appreciated! LiveTraffic_3.1.2.zip

EDIT: Tried hard to get X-Plane 12 on Mac to display German numerals...in vain. My system settings are set to German all the time. Have set LANG and LC_NUMERIC even in a terminal and started X-Plane from the terminal...none of that produced German-looking numbers within X-Plane, although I would expect e.g. latitutde/longitude figures to then show up with command instead of dot as decimal separator...but they don't. So I still can't reproduce your actual scenario.

When you run with German numerals setting, will decimal numbers in LiveTraffic appear in German notation, e.g. in the Aircraft List or the Info Window? (From a full Log.txt, that neither of you provided, I could have probably figures out myself from log entries with decimal numbers...I will never get tired of repeating like a broken record that attaching a full Log.txt would often help way beyond the one line that everybody looks it.)

sfrsfrsfr commented 2 years ago

Hi TwinFan/LiveTraffic,

i can confirm your concern regarding the JSON parsing errors.

LiveTraffic: Current System time is 2022-11-05 18:28:42Z, current simulated time is 2022-11-05 18:27:12.0Z 0:01:22,210 LiveTraffic ERROR LTOpenSky.cpp:180/ProcessFetchedData: Parsing flight data as JSON failed 0:01:22,514 I/PLG: The plugin LiveTraffic 3.1.2 is setting global TCAS override to 1. 0:01:33,420 E/ATC: Radio transmission required but no voice loaded! 0:01:42,123 LiveTraffic ERROR LTOpenSky.cpp:180/ProcessFetchedData: Parsing flight data as JSON failed

See also the attached full Log.txt.

My locale settings:

locale LANG=en_US.UTF-8 LANGUAGE= LC_CTYPE="en_US.UTF-8" LC_NUMERIC=de_DE.UTF-8 LC_TIME=de_DE.UTF-8 LC_COLLATE="en_US.UTF-8" LC_MONETARY=de_DE.UTF-8 LC_MESSAGES="en_US.UTF-8" LC_PAPER=de_DE.UTF-8 LC_NAME=de_DE.UTF-8 LC_ADDRESS=de_DE.UTF-8 LC_TELEPHONE=de_DE.UTF-8 LC_MEASUREMENT=de_DE.UTF-8 LC_IDENTIFICATION=de_DE.UTF-8 LC_ALL=

Thx for looking into this. Fyi, setting LC_NUMERIC to us_US.UTF-8 in a small wrapper script works just fine for me.

Bye, Stefan

On Fri, Nov 4, 2022 at 11:45 PM TwinFan @.***> wrote:

@sfrsfrsfr https://github.com/sfrsfrsfr , can you try the attached version to see if that fixes your problem. I am pretty sure that the correct URL gets produced. I'm just a bit concerned that down the road you might end up with JSON parsing errors as that is what happened during my testing. Not totally unreasonable...the locale will influence interpreting strings as lists and numbers, too. But I'm not on Linux and the way I overruled the locale might not have been proper.

Feedback appreciated! LiveTraffic_3.1.2.zip https://github.com/TwinFan/LiveTraffic/files/9942391/LiveTraffic_3.1.2.zip

— Reply to this email directly, view it on GitHub https://github.com/TwinFan/LiveTraffic/issues/244#issuecomment-1304316950, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQIPJBBY4AVMB3EG4D46UTDWGWGX7ANCNFSM6AAAAAARSE73OM . You are receiving this because you were mentioned.Message ID: @.***>

TwinFan commented 2 years ago

Out of curiosity I've taken a look into the Parson library I use for JSON parsing: It uses many hard-coded characters like , for field separation, as per JSON standard, and then strtod for parsing numbers, which is locale-dependent, and that's where parsing breaks. But replacing those strings before parsing is a sure path to peril...

So I've looked into thread-local locales once again, and indeed there are options. I've used this Stackoverflow suggestion now.

My test on Mac (I had a debug statement in the code that set the global(!) locale to "de_DE" when loading the plugin) worked out now. All communication threads where affected, e.g. also reading the weather...

With the attached new version I could finally produce German numbers in the UI while still successfully loading and parsing data (need to watch the decimal comma in the OGN line ;-) )

LT_DE_Locale

So, @sfrsfrsfr, please try this one: LiveTraffic_3.1.2.zip

sfrsfrsfr commented 2 years ago

Hi TwinFan/LiveTraffic,

this doesn't seem to work on Ubuntu 22.04, X-Plane 12 beta 11. It again generates OpenSky URLs with , instead of . Log attached.

Bye, Stefan

On Sun, Nov 6, 2022 at 12:01 AM TwinFan @.***> wrote:

Out of curiosity I've taken a look into the Parson library I use for JSON parsing: It uses many hard-coded characters like , for field separation, as per JSON standard, and then strtod for parsing numbers, which is locale-dependent, and that's where parsing breaks. But replacing those strings before parsing is a sure path to peril...

So I've looked into thread-local locales once again, and indeed there are options. I've used this Stackoverflow suggestion https://stackoverflow.com/a/17173977 now.

My test on Mac (I had a debug statement in the code that set the global(!) locale to "de_DE" when loading the plugin) worked out now. All communication threads where affected, e.g. also reading the weather...

With the attached new version I could finally produce German numbers in the UI while still successfully loading and parsing data (need to watch the decimal comma in the OGN line ;-) ) [image: LT_DE_Locale] https://user-images.githubusercontent.com/44291093/200144456-6c72260c-8d40-4054-a366-7443d33309ff.png

So, @sfrsfrsfr https://github.com/sfrsfrsfr, please try this one: LiveTraffic_3.1.2.zip https://github.com/TwinFan/LiveTraffic/files/9944840/LiveTraffic_3.1.2.zip

— Reply to this email directly, view it on GitHub https://github.com/TwinFan/LiveTraffic/issues/244#issuecomment-1304658910, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQIPJBCEURXOXAEOYPS7FALWG3RNVANCNFSM6AAAAAARSE73OM . You are receiving this because you were mentioned.Message ID: @.***>

TwinFan commented 2 years ago

:-( Unfortunately, attaching files to issues doesn’t work when replying by mail. Can you attach the mailed Log.txt file directly on GitHub?

sfrsfrsfr commented 2 years ago

sure, it should be available now Log.txt.gz

TwinFan commented 2 years ago

From your log file I see that your German locale also affects X-Plane system functions:

0:00:56,085 W/ATC: Limiting EDGG Area Control Center (118,750, 119,110, 119,150, 1

These are frequencies. With English format that would read 118.750, 119.110.... So your locale does affect -as expected- the entire XP system.

I've now (for this, but also for another Linux issue) set up my own Ubuntu system, installed XP12b11 and LiveTraffic, actually a build even without the "fix" discussed here...and I can still not reproduce the issue, though with a system with English language and German regional settings I have pretty exactly the same locale as you:

Charlie-lin:~/X-Plane/12$ uname -a
Linux Charlie-lin 5.15.0-52-generic #58-Ubuntu SMP Thu Oct 13 08:03:55 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Charlie-lin:~/X-Plane/12$ locale
LANG=en_US.UTF-8
LANGUAGE=en_US:en
LC_CTYPE="en_US.UTF-8"
LC_NUMERIC=de_DE.UTF-8
LC_TIME=de_DE.UTF-8
LC_COLLATE="en_US.UTF-8"
LC_MONETARY=de_DE.UTF-8
LC_MESSAGES="en_US.UTF-8"
LC_PAPER=de_DE.UTF-8
LC_NAME=de_DE.UTF-8
LC_ADDRESS=de_DE.UTF-8
LC_TELEPHONE=de_DE.UTF-8
LC_MEASUREMENT=de_DE.UTF-8
LC_IDENTIFICATION=de_DE.UTF-8
LC_ALL=
Charlie-lin:~/X-Plane/12$ ./X-Plane-x86_64 

What I see in X-Plane is US-formated numbers (ie. using a dot) and fully functional LiveTraffic:

0:01:47.684 W/ATC: Limiting EDUU Area Control Center (118.215, 120.930, 121.440, 122.635, 124.035, ...

I am not saying the problem doesn't exist...two individuals reported it...but I still can't get there.

TwinFan commented 2 years ago

How to you actually start X-Plane? And how does your wrapper script look like.

I start X-Plane from the command line like shown above:

Charlie-lin:~/X-Plane/12$ ./X-Plane-x86_64

or by double-clicking that executable in a file explorer window.

sfrsfrsfr commented 2 years ago

the wrapper script "xplane.sh" is very simple:

#!/bin/sh

# 2x FHD
# --monitor_bounds=0,0,1920,1080,1920,0,1920,1080
#
# 1x WQHD, 1x FHD
# --monitor_bounds=0,0,2560,1440,1920,0,1920,1080

CWD=`pwd`
export LC_NUMERIC="en_US.UTF-8"

echo "1 is $1"
echo "2 is $2"
echo "3 is $3"

cd $HOME/X-Plane\ 12
./X-Plane-x86_64 $1 $2 $3

cd $CWD

i use i3wm and run the wrapper in a terminal

TwinFan commented 1 year ago

A Windows(!) user on XP11(!) now reported something very similar: errors while parsing (here: RealTraffic) tracking data and wrong URLs for requesting data from the internet (here: weather information). See here on the Org.

No feedback as of yet (but hey, it‘s Christmas) if the suggestion to change number formatting on Windows-level helped. But offers the chance for me to try on Windows, which I hadn’t done before.

TwinFan commented 1 year ago

Quote from the support thread, analysis that I am suspecting another plugin the interfere with the process' locale:

On a different note: I have compared the log your posted first with the one posted now and I noticed something interesting, which becomes apparent in these lines of the first log from yesterday:

0:00:00.000 Altitude/win/XPMP2 INFO CSLModels.cpp:1091/CSLModelsLoad: Total number of known models now is 5370 ... 0:00:50.308 LiveTraffic INFO LiveTraffic.cpp:420/LoopCBOneTimeSetup: LiveTraffic 3.2.2 (19-Dec-2022) successfully loaded! 0:00:50,308 LiveTraffic INFO LTApt.cpp:2576/LTAptRefresh: Starting thread to read apt.dat for airports 120,0nm around 82,4999 N / 82,9999 E 0:00:50,308 Altitude/win/XPMP2 DEBUG Map.cpp:338/MapLayerCreate: Created a map layer for map 'XPLM_MAP_IOS'

Only look at the timestamp at the very beginning: Both Altitude (which uses the same XPMP2 library as LiveTraffic) and LiveTraffic originally format the timestamp with a dot to separate the fractions of a second from the seconds, but then suddenly, in the mids of processing, that change to a comma!

I can assure you that neither LiveTraffic nor XPMP2 have any code to influence the regional formatting or the locale.

My guess is that some other plugin, noting the Spanish regional settings, sets the locale. But if done the straight-forward simple way, then the locale changes for the entire process, i.e. all plugins. It might be worthwhile to change back to Spanish number settings, and then try to remove one of the other plugins one by one to see which one causes the locale change.

TwinFan commented 1 year ago

Based on the above idea I created another plugin, with which I can switch the process locale (using std::setlocale()) between "C" and "de_DE.UTF-8". And this way I was able to reproduce the issue and can also explain why my initial fixing attempt above did not work: It had a totally stupid coding bug... :-(

TwinFan commented 1 year ago

@sfrsfrsfr, can you do me a favour a try this attached version without you setting LC_NUMERIC in your startup script. LiveTraffic_3.3.0_lin.zip

sfrsfrsfr commented 1 year ago

i'll try the latest version real soon (today/tomorrow) and will let you know. I appreciate your effort on this.

On Sat, Dec 31, 2022 at 2:36 PM TwinFan @.***> wrote:

Closed #244 https://github.com/TwinFan/LiveTraffic/issues/244 as completed via b6fb7bb https://github.com/TwinFan/LiveTraffic/commit/b6fb7bb7f3898d155e23ec71c44d7ba684dd74b7 .

— Reply to this email directly, view it on GitHub https://github.com/TwinFan/LiveTraffic/issues/244#event-8132690929, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQIPJBAELMP4C6H2LRBEHRDWQAZETANCNFSM6AAAAAARSE73OM . You are receiving this because you were mentioned.Message ID: @.***>

sfrsfrsfr commented 1 year ago

i can confirm 3.3.0a fixed the locale issue, thx