Possible case-sensitivity issue in module-link

Possible case-sensitivity issue in module-link

[Gianni Ceccarelli]

Hello!

I use cgit on my own site https://www.thenautilus.net/cgit/

Some of my repositories contain submodules, for example
https://www.thenautilus.net/cgit/Sietima/tree/docs/presentation/
https://www.thenautilus.net/cgit/thermostat/tree/
https://www.thenautilus.net/cgit/thermostat/tree/sensor/
https://www.thenautilus.net/cgit/lego-piano/tree/
https://www.thenautilus.net/cgit/lego-piano/tree/3d-print

All those submodules are listed in the repo's config file, for
example ``thermostat/config`` contains::

  [cgit "module-link"]
    esp8266-oled-ssd1306 = https://github.com/ThingPulse/esp8266-oled-ssd1306/tree/%s
    DHTesp = https://github.com/beegee-tokyo/DHTesp/tree/%s
    bt-server = https://www.thenautilus.net/cgit/gobbledegook/tree/?id=%s

and ``lego-piano/config`` contains::

  [cgit "module-link"]
    RingBuffer = https://github.com/Locoduino/RingBuffer/tree/%s
    ESP8266Audio = https://github.com/earlephilhower/ESP8266Audio/tree/%s
  [cgit "module-link.3d-print/LEGO"]
    scad = https://github.com/cfinke/LEGO.scad/tree/%s

You may have noticed that only *some* submodules show up as links in
the CGit output:

- ``esp8266-oled-ssd1306``, ``bt-server``, ``LEGO.scad`` work
- ``DHTesp``, ``RingBuffer``, ``ESP8266Audio`` don't

My suspicion is that the case of the characters after the last ``.``
is involved: if they're all lowercase, the link works, if some are
uppercase, it doesn't.

Running ``git config -l`` in the ``lego-piano`` repository directory,
for example, I get::

  cgit.module-link.ringbuffer=https://github.com/Locoduino/RingBuffer/tree/%s
  cgit.module-link.esp8266audio=https://github.com/earlephilhower/ESP8266Audio/tree/%s
  cgit.module-link.3d-print/LEGO.scad=https://github.com/cfinke/LEGO.scad/tree/%s

notice that ``ringbuffer`` and ``esp8266audio`` got lowercased, but
``LEGO`` didn't. If the configuration is parsed like this, and the
submodule's path is then matched case-sensitively, the result would be
what I observe.

Can anyone confirm? I've been reading the source of CGit plus the bits
of Git that are linked in, but I haven't found code that would produce
the behaviour I see.

Thanks in advance.

-- 
	Dakkar - <Mobilis in mobile>
	GPG public key fingerprint = A071 E618 DD2C 5901 9574
	                             6FE2 40EA 9883 7519 3F88
	                    key id = 0x75193F88

Re: Possible case-sensitivity issue in module-link

[John Keeping]

On Sat, Aug 29, 2020 at 12:03:49PM +0100, Gianni Ceccarelli wrote:
> I use cgit on my own site https://www.thenautilus.net/cgit/
> 
> Some of my repositories contain submodules, for example
> https://www.thenautilus.net/cgit/Sietima/tree/docs/presentation/
> https://www.thenautilus.net/cgit/thermostat/tree/
> https://www.thenautilus.net/cgit/thermostat/tree/sensor/
> https://www.thenautilus.net/cgit/lego-piano/tree/
> https://www.thenautilus.net/cgit/lego-piano/tree/3d-print
> 
> All those submodules are listed in the repo's config file, for
> example ``thermostat/config`` contains::
> 
>   [cgit "module-link"]
>     esp8266-oled-ssd1306 = https://github.com/ThingPulse/esp8266-oled-ssd1306/tree/%s
>     DHTesp = https://github.com/beegee-tokyo/DHTesp/tree/%s
>     bt-server = https://www.thenautilus.net/cgit/gobbledegook/tree/?id=%s
> 
> and ``lego-piano/config`` contains::
> 
>   [cgit "module-link"]
>     RingBuffer = https://github.com/Locoduino/RingBuffer/tree/%s
>     ESP8266Audio = https://github.com/earlephilhower/ESP8266Audio/tree/%s
>   [cgit "module-link.3d-print/LEGO"]
>     scad = https://github.com/cfinke/LEGO.scad/tree/%s
> 
> You may have noticed that only *some* submodules show up as links in
> the CGit output:
> 
> - ``esp8266-oled-ssd1306``, ``bt-server``, ``LEGO.scad`` work
> - ``DHTesp``, ``RingBuffer``, ``ESP8266Audio`` don't
> 
> My suspicion is that the case of the characters after the last ``.``
> is involved: if they're all lowercase, the link works, if some are
> uppercase, it doesn't.
> 
> Running ``git config -l`` in the ``lego-piano`` repository directory,
> for example, I get::
> 
>   cgit.module-link.ringbuffer=https://github.com/Locoduino/RingBuffer/tree/%s
>   cgit.module-link.esp8266audio=https://github.com/earlephilhower/ESP8266Audio/tree/%s
>   cgit.module-link.3d-print/LEGO.scad=https://github.com/cfinke/LEGO.scad/tree/%s
> 
> notice that ``ringbuffer`` and ``esp8266audio`` got lowercased, but
> ``LEGO`` didn't. If the configuration is parsed like this, and the
> submodule's path is then matched case-sensitively, the result would be
> what I observe.
> 
> Can anyone confirm? I've been reading the source of CGit plus the bits
> of Git that are linked in, but I haven't found code that would produce
> the behaviour I see.

Yes, Git's config parsing converts all keys to lowercase (see
git/config.c::get_value() and its caller).

Does the patch below fix the bug?

-- >8 --
Subject: [PATCH] shared: compare submodules case-insensitively

If module-link is read from a repository's Git config, then our callback
is invoked with the name converted to lower case (as all Git config keys
are).  This means we can never match a submodule containing uppercase
characters against such a module-link.

Compare submodule names case-insensitively to avoid this problem.  This
may break a use case where two submodules differ only in case, but that
is much less likely than a submodule name using uppercase characters.

Signed-off-by: John Keeping <john@keeping.me.uk>
---
 shared.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/shared.c b/shared.c
index 609bd2a..777618d 100644
--- a/shared.c
+++ b/shared.c
@@ -77,6 +77,7 @@ struct cgit_repo *cgit_add_repo(const char *url)
 	ret->owner_filter = ctx.cfg.owner_filter;
 	ret->clone_url = ctx.cfg.clone_url;
 	ret->submodules.strdup_strings = 1;
+	ret->submodules.cmp = strcasecmp;
 	ret->hide = ret->ignore = 0;
 	return ret;
 }
-- 
2.28.0

Re: Possible case-sensitivity issue in module-link

[Gianni Ceccarelli]

On 2020-08-29 John Keeping <john@keeping.me.uk> wrote:
> Yes, Git's config parsing converts all keys to lowercase (see
> git/config.c::get_value() and its caller).
> 
> Does the patch below fix the bug?

It does, thank you!

-- 
	Dakkar - <Mobilis in mobile>
	GPG public key fingerprint = A071 E618 DD2C 5901 9574
	                             6FE2 40EA 9883 7519 3F88
	                    key id = 0x75193F88