Using a library to parse config files

[TOMATO-ONE]

Hi,all

I am using xrdp as RDP Proxy.
I have added a lot of settings for RDP Proxy in xrdp.ini.
I am modifying xrdp.ini when new xrdp is released.

Is it possible to put some of the xrdp.ini settings under /etc/xrdp/conf.d/ and include them, similar to apache and nginx?

If that is possible, I could put the configuration regarding RDP Proxy in conf.d.
As another application, when adding the xorgxrdp rpm/deb package, We could add the configuration for Xorg in conf.d, which would make it easier for the package manager.

[metalefty]

I'm in positive position with that if it is not hard to implement and will not make code very complex. It is a good chance to add such feature because we're switching to v1.0.

[matt335672]

Sounds like a good idea.

We need to define behaviour in the event of colliding sections. So for example, if we used include as a keyword and /etc/xrdp/xrdp.ini looked like this:-

[section 1]
aaa=bbb
ccc=ddd

include /etc/conf.d/xrdp.conf

and /etc/conf.d/xrdp.conf looked like this:-

[section 1]
ccc=eee

I'd expect the result to be either-

[section 1]
aaa=bbb
ccc=eee

or:-

[section 1]
ccc=eee

I could make a case for either one. What do people think?

[matt335672]

I like that, and what's more it's easy to do.

We've got a function g_format_info_string() which allows for strings to be parsed for exactly this purpose.

What do you think @TOMATO-ONE?

[TOMATO-ONE]

Thank you. @metalefty

Great idea!
I see that there is also a way to put it in the /etc/hosts file on the VNC/RDP-Proxy host without having to rely on DNS.

By the way.
This topic is a derivative one, so I think it's better to start a separate discussion.
What do you think, @matt335672 , @metalefty ?

[metalefty]

Agreed.

[matt335672]

Could you raise an issue on this @TOMATO-ONE? We can then mark it as a feature request. I think we're agreed it's a useful thing to have.

I think your original thread has been somewhat hijacked - apologies. I'll change the title of this one.

[TOMATO-ONE]

I have created a new discussion #2410.
I will continue the discussion here.

@matt335672
I will create a new issue later.

[metalefty]

Also, it is a good chance to bump ini_version for v1.0. It indicates config files need migration.

Furthermore, I would personally like to switch configuration file format to TOML. INI is a mature config file format but AFAIK there's no specification. In contrast, TOML has a formal specification. Another advantage of TOML is the existence of a parser library. We don't need to implement our own config parser.

Let's research about TOML parser library for C language available on each OS on this thread.

[matt335672]

I've had a quick look at TOML.

Initial comments:-

• Case-sensitive keys will cause a compatibility problem. The existing parser uses case insensitivity.
• include files are not supported directly in v1.0.0. I was hoping for something like that to implement this feature!
• It will make it much easier to read xrdp configs from other languages and utilities which could be useful.
• (edit) Only '#' is supported for comments

As regarding platform support, I can't find anything for C in FreeBSD ports. Debian 10 has support for Go, Rust, Perl, Python and Ruby but not C. CentOS 7 has Go and Python, but no C. Am I missing something?

[metalefty]

As far as I looked, very few C libraries for TOML are available on most platforms. It seems that TOML is popular in script language but not in C language so far :(. toml11 is available on most platform but it is for C++.

What about keeping using INI but with a library instead? iniparser is available on most platforms. Arch, CentOS, Debian and FreeBSD.

[matt335672]

**11:12 Updated **
I've had a look at libiniparser, and also inih which is available on the same platforms.

This is a static analysis only. I've not tried implementing anything.

Supported syntax

Both support ';' and '#' for comments.

• iniparser supports mixed-case keys. However, the case of the original key is lost. This is incompatible with bits of xrdp.ini where we use the case of the key for (e.g.) "Password=".
• iniparser only supports one key per section. Multiple lines with the same key are silently ignored. This is incompatible with the param= lines in sesman.ini.
• iniparser supports Y/y/T/t/1 for boolean true and N/n/F/f/0 for boolean false. This is slightly different from our g_text2bool() call.
• iniparser supports hexadecimal integers with an 0x prefix and octal integers with an 0 prefix.
• inih supports all existing features, as it's so simple!

Error reporting

iniparser uses a callback, whereas inih essentially just stops parsing with the error line returned.

Maintenance

• Both are under permissive licenses which would be useful if we needed to embed one within xrdp for maintenance reasons.
• Both projects show some activity in the last year. You wouldn't expect a lot of activity on a project like this!
• iniparser has a more complete API. This means less code on our part, but also means more effort if the API changes. It hasn't for at least 5 years.

Popularity

Popularity; both are used by a few other packages only in Debian11:-

mjb@debian11:~$ sudo apt-cache rdepends libiniparser1
libiniparser1
Reverse Depends:
  libiniparser-dev
  mtd-utils
  renderd
  cava
mjb@debian11:~$ sudo apt-cache rdepends libinih1
libinih1
Reverse Depends:
  libinih-dev
  xfsprogs
  libretro-mgba
  libmgba
  libinireader0
  gamemode-daemon
  imv

Other information

I see that iniparser also comes with a rudimentary hash table which we could maybe use elsewhere.

Conclusion

Both projects are viable, compatible with xrdp, and look like they could be used to replace our existing parser, but could introduce incompatibilities with existing files.

Personally I'd prefer iniparser I think because of the poor error reporting in inih. It also provides more built-in functionality. The main worry I'd have with it is the handling of param= lines in sesman.ini, although we also need a way to handle the case of prompts on the xrdp login screen.

Thoughts?

[metalefty]

Thanks for the detailed research. One more thought regarding TOML. I think we can embed third-party library tomlc99 in xrdp source as a submodule. As far as I searched on GitHub, I found many projects depend on tomlc99 and they embed it in their own source.

https://github.com/search?q=tomlc99+submodule&type=Code

[matt335672]

Personally I'd be happy with that approach. The licence looks OK. There's a couple of possible wrinkles:-

• We're insisting on a compiler that can handle C99, and probably the switches from the Makefile. I'm happy with that, but Jay may not be - see for example build fix for older gcc #1722.
• We'll need to disable code formatting checks for that directory. That should be easily doable.
• We'll probably need to wrap the directory to interface with the Makefile it uses

It seems to me we now have three possible approaches to changing the parser.

• Use libh and stick with the existing formats. Advantages; no user action required, and least change. Disadvantages; we still have hard-to-parse config files. Error reporting is minimal.
• Use iniparser and refuse to accept ini files which haven't been updated. Advantages; Uses a tried and tested library. Files are easier to parse. Disadvantages; Some user impact which needs to be managed.
• Use toml99. Advantages; Easiest to parse. Disadvantages; Need to integrate with xrdp build and CI systems. C99 is forced on us. User impact is greatest - we may need to provide a migration utility.

Another thing to bear in mind is the original discussion point, which is to read the config from multiple files, if I'm correctly understanding it. This is somewhat at odds with making the configs easier to parse.

Is that a reasonable summary?

@jsorg71 - what do you think about this discussion so far?

[jsorg71]

@matt335672 I been assuming you guys are already defaulting to C99.
I don't have a strong opinion here. I did try toml99 and iniparser and both seem to compile on everything I can think to try.

[matt335672]

@jsorg71 - we're currently on C90 (with some exceptions for your SPARC machine). POSIX (IEEE Std 1003.1-2017) mentions C99, so it would seem to be a good place to aim for. Our current cppcheck CI test uses C11.

Are you still using the SPARC machine for big-endian testing? There seems to be very little big-endian hardware out there these days.

[jsorg71]

@matt335672 This is getting a bit off topic but what exceptions for SPARC are you talking about? Are you sure it's not more about an older GCC?
It is true that there seems to be less big endian systems these days. PowerPC can run big or little I think. RISC-V I think is little endian only. There is MIPS as well. I think we should support big endian as long as major distros support.
What is the problem with supporting big endian? I think we should start a new discussion if this important and keep this about the config library.

[matt335672]

Platform discussion moved to #2441