Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to delete entry containing dots with config:delete command #19966

Closed
davidlemaitre opened this issue Nov 7, 2022 · 2 comments · Fixed by #20764
Closed

Unable to delete entry containing dots with config:delete command #19966

davidlemaitre opened this issue Nov 7, 2022 · 2 comments · Fixed by #20764
Labels
Bug For errors / faults / flaws / inconsistencies etc. Needs priority decision This issue may need to be added to the current milestone by Product Manager
Milestone

Comments

@davidlemaitre
Copy link

I want to use config:delete command to remove an ip address from General.login_allowlist_ip array but the command never found the value. If I add a new value without dots to an array, the deletion works. I tried to escape dots with backslashes without success.

Steps to Reproduce

./console config:set 'General.login_allowlist_ip[]="123.123.123.123"'
Setting [General] login_allowlist_ip = "123.123.123.123"... done.
./console config:get 'General.login_allowlist_ip'`
["123.123.123.123"]
./console config:delete --section=General --key=login_allowlist_ip[] --value=123.123.123.123`
Nothing found

Your Environment

  • Matomo Version: 4.12.3
  • PHP Version: 8.0.25
@davidlemaitre davidlemaitre added the Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. label Nov 7, 2022
@peterhashair
Copy link
Contributor

peterhashair commented Nov 7, 2022

@davidlemaitre thanks for reporting this, you are right, I can see there is a bug there caused by the IP address having dot in it. Instead of searching for 123.123.123.123, it searches 123

@peterhashair peterhashair added Bug For errors / faults / flaws / inconsistencies etc. Needs priority decision This issue may need to be added to the current milestone by Product Manager and removed Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. labels Nov 7, 2022
@peterhashair
Copy link
Contributor

Hints: this is caused by this line.

if (!preg_match('/^([a-zA-Z0-9_]+)(?:\.([a-zA-Z0-9_]+))?(?:\[\])?(?:\.([a-zA-Z0-9_]+))?/', $settingStr, $matches) || empty($matches[1])) {

possible solution:

update the regex to /^([a-zA-Z0-9_]+)(?:\.([a-zA-Z0-9_]+))?(?:\[\])?(?:\.([a-zA-Z0-9_.]+))?/, not sure if we want to accept all special characters

ziegenberg added a commit to ziegenberg/matomo that referenced this issue May 19, 2023
…ig:delete to config:set

The console command `config:set` allows any character for the `--value`
option (see:
`\Piwik\Plugins\CoreAdminHome\Commands\SetConfig\ConfigSettingManipulation::make`).

The regex used for `config:set` is:
`/^([a-zA-Z0-9_]+)\.([a-zA-Z0-9_]+)(\[\])?=(.*)/`

The console command `config:delete` only allows characters a to z (upper
and lower case), numbers and the underscore. This prevents one from
deleting configs values like IPv4 or IPV6 addresses, mail addresses,
networks or IP ranges and lists of values. The currently used regex is:
`/^([a-zA-Z0-9_]+)(?:\.([a-zA-Z0-9_]+))?(?:\[\])?(?:\.([a-zA-Z0-9_]+))?/`

The following is a list of possible settings found in the wild. Those
can be set with `console config:set` but cannot be deleted.

```
[General]
login_allowlist_ip[] = "123.123.123.123"
trusted_hosts = "[2001:858:6::186]:80"
contact_email_address = "no-reply+with-extension@test.at"
proxy_ips[] = "192.168.x.x/24"

[HeatmapSessionRecording]
session_recording_sample_limits = 50,100,250,500,1000,2000,5000
```

This commit aligns the allowed characters for the option `--value` on
the console command `config:delete` to the ones for `config:set` and
changes the regex to
`/^([a-zA-Z0-9_]+)(?:\.([a-zA-Z0-9_]+))?(?:\[\])?(?:\.(.+))?/`.

Fixes: matomo-org#19966

Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
sgiehl pushed a commit to ziegenberg/matomo that referenced this issue Jun 20, 2023
…ig:delete to config:set

The console command `config:set` allows any character for the `--value`
option (see:
`\Piwik\Plugins\CoreAdminHome\Commands\SetConfig\ConfigSettingManipulation::make`).

The regex used for `config:set` is:
`/^([a-zA-Z0-9_]+)\.([a-zA-Z0-9_]+)(\[\])?=(.*)/`

The console command `config:delete` only allows characters a to z (upper
and lower case), numbers and the underscore. This prevents one from
deleting configs values like IPv4 or IPV6 addresses, mail addresses,
networks or IP ranges and lists of values. The currently used regex is:
`/^([a-zA-Z0-9_]+)(?:\.([a-zA-Z0-9_]+))?(?:\[\])?(?:\.([a-zA-Z0-9_]+))?/`

The following is a list of possible settings found in the wild. Those
can be set with `console config:set` but cannot be deleted.

```
[General]
login_allowlist_ip[] = "123.123.123.123"
trusted_hosts = "[2001:858:6::186]:80"
contact_email_address = "no-reply+with-extension@test.at"
proxy_ips[] = "192.168.x.x/24"

[HeatmapSessionRecording]
session_recording_sample_limits = 50,100,250,500,1000,2000,5000
```

This commit aligns the allowed characters for the option `--value` on
the console command `config:delete` to the ones for `config:set` and
changes the regex to
`/^([a-zA-Z0-9_]+)(?:\.([a-zA-Z0-9_]+))?(?:\[\])?(?:\.(.+))?/`.

Fixes: matomo-org#19966

Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
sgiehl pushed a commit that referenced this issue Jun 20, 2023
…fig:delete` to `config:set` (#20764)

* align allowed characters for option '--value' on console command config:delete to config:set

The console command `config:set` allows any character for the `--value`
option (see:
`\Piwik\Plugins\CoreAdminHome\Commands\SetConfig\ConfigSettingManipulation::make`).

The regex used for `config:set` is:
`/^([a-zA-Z0-9_]+)\.([a-zA-Z0-9_]+)(\[\])?=(.*)/`

The console command `config:delete` only allows characters a to z (upper
and lower case), numbers and the underscore. This prevents one from
deleting configs values like IPv4 or IPV6 addresses, mail addresses,
networks or IP ranges and lists of values. The currently used regex is:
`/^([a-zA-Z0-9_]+)(?:\.([a-zA-Z0-9_]+))?(?:\[\])?(?:\.([a-zA-Z0-9_]+))?/`

The following is a list of possible settings found in the wild. Those
can be set with `console config:set` but cannot be deleted.

```
[General]
login_allowlist_ip[] = "123.123.123.123"
trusted_hosts = "[2001:858:6::186]:80"
contact_email_address = "no-reply+with-extension@test.at"
proxy_ips[] = "192.168.x.x/24"

[HeatmapSessionRecording]
session_recording_sample_limits = 50,100,250,500,1000,2000,5000
```

This commit aligns the allowed characters for the option `--value` on
the console command `config:delete` to the ones for `config:set` and
changes the regex to
`/^([a-zA-Z0-9_]+)(?:\.([a-zA-Z0-9_]+))?(?:\[\])?(?:\.(.+))?/`.

Fixes: #19966

Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>

* extend and fix tests for console command config:delete

Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>

---------

Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
@sgiehl sgiehl modified the milestones: 4.15.0, 5.0.0 Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc. Needs priority decision This issue may need to be added to the current milestone by Product Manager
Projects
None yet
3 participants