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

Twitter V2 bridge not able to load settings from environment #2935

Closed
dot-mike opened this issue Jul 23, 2022 · 6 comments · Fixed by #2973
Closed

Twitter V2 bridge not able to load settings from environment #2935

dot-mike opened this issue Jul 23, 2022 · 6 comments · Fixed by #2973
Labels
Bug-Report Confirmed bug report

Comments

@dot-mike
Copy link

Describe the bug
Unable to set setting for Twitter V2 bridge using environment keys due to configuration forcing keys to be lower-case
In my docker-compose file I set the environment from docker-compose file, then try to use Twitter V2 bridge I get an error

Uncaught Exception Exception: Missing configuration option: twitterv2apitoken at lib/error.php line 26

#0 index.php:32
#1 actions/DisplayAction.php:41
#2 lib/BridgeAbstract.php:293
#3 lib/error.php:46
#4 lib/error.php:26 

Docker-compose file for reference:

version: '3.9'
services:
  rssbridge:
    image: rssbridge/rss-bridge:latest
    restart: unless-stopped
    environment:
      - RSSBRIDGE_TwitterV2Bridge_twitterv2apitoken=MySuperTopSecretTwitterApiToken

To Reproduce
Steps to reproduce the behavior:

  1. Use my docker compose file
  2. Whitelist twitterv2 brdige
  3. Try to use twitterv2
  4. See error

Additional context
The cause of this is the fact that Configuration.php sets the section-name to all lower-case.
From my log file where I enabled some debugging for environmental keys.

2022-07-23T01:33:02.149345228Z 127.0.0.1 -  23/Jul/2022:03:33:01 +0200 "GET /index.php" 200
2022-07-23T01:36:21.505756069Z NOTICE: PHP message: /app/index.php:6 class Configuration->loadConfiguration - PUID
2022-07-23T01:36:21.505789862Z NOTICE: PHP message: /app/index.php:6 class Configuration->loadConfiguration - HOSTNAME
2022-07-23T01:36:21.505794852Z NOTICE: PHP message: /app/index.php:6 class Configuration->loadConfiguration - PHP_VERSION
2022-07-23T01:36:21.505798681Z NOTICE: PHP message: /app/index.php:6 class Configuration->loadConfiguration - PHP_INI_DIR
2022-07-23T01:36:21.505802162Z NOTICE: PHP message: /app/index.php:6 class Configuration->loadConfiguration - GPG_KEYS
2022-07-23T01:36:21.505813547Z NOTICE: PHP message: /app/index.php:6 class Configuration->loadConfiguration - PHP_LDFLAGS
2022-07-23T01:36:21.505816983Z NOTICE: PHP message: /app/index.php:6 class Configuration->loadConfiguration - RSSBRIDGE_TwitterV2Bridge_twitterv2apitoken
2022-07-23T01:36:21.505820710Z NOTICE: PHP message: /app/index.php:6 class Configuration->loadConfiguration - Got settings from env. Section: twitterv2bridge key: twitterv2apitoken = MySuperTopSecretTwitterApiToken
2022-07-23T01:36:21.505824284Z NOTICE: PHP message: /app/index.php:6 class Configuration->loadConfiguration - PWD
2022-07-23T01:36:21.505827958Z NOTICE: PHP message: /app/index.php:6 class Configuration->loadConfiguration - HOME
2022-07-23T01:36:21.505831324Z NOTICE: PHP message: /app/index.php:6 class Configuration->loadConfiguration - PGID
2022-07-23T01:36:21.505834466Z NOTICE: PHP message: /app/index.php:6 class Configuration->loadConfiguration - PHP_SHA256
2022-07-23T01:36:21.505837845Z NOTICE: PHP message: /app/index.php:6 class Configuration->loadConfiguration - PHPIZE_DEPS
2022-07-23T01:36:21.505841138Z NOTICE: PHP message: /app/index.php:6 class Configuration->loadConfiguration - PHP_URL
2022-07-23T01:36:21.505844288Z NOTICE: PHP message: /app/index.php:6 class Configuration->loadConfiguration - SHLVL
2022-07-23T01:36:21.505847606Z NOTICE: PHP message: /app/index.php:6 class Configuration->loadConfiguration - PHP_CFLAGS
2022-07-23T01:36:21.505850732Z NOTICE: PHP message: /app/index.php:6 class Configuration->loadConfiguration - PATH
2022-07-23T01:36:21.505853987Z NOTICE: PHP message: /app/index.php:6 class Configuration->loadConfiguration - PHP_ASC_URL
2022-07-23T01:36:21.505857199Z NOTICE: PHP message: /app/index.php:6 class Configuration->loadConfiguration - PHP_CPPFLAGS
2022-07-23T01:36:21.505860387Z NOTICE: PHP message: /app/index.php:6 class Configuration->loadConfiguration - _
2022-07-23T01:36:21.505864092Z NOTICE: PHP message: /app/index.php:6 class Configuration->loadConfiguration - USER
2022-07-23T01:36:21.507688793Z NOTICE: PHP message: /app/actions/DisplayAction.php:41 class BridgeAbstract->loadConfiguration - Config section TwitterV2Bridge does not exists for key twitterv2apitoken
2022-07-23T01:36:21.507702656Z NOTICE: PHP message: Exception: Missing configuration option: twitterv2apitoken in /app/lib/error.php:26
@dot-mike dot-mike added the Bug-Report Confirmed bug report label Jul 23, 2022
@dot-mike
Copy link
Author

For reference:
as far as I can tell, when a bridge is instanced, loadConfiguration is called which again calls getConfig for the specific bridge that check if a config section exist for the specific bridge within the loaded configuration. Since loaded environmental keys are forced to lower case and getConfig newer transforms the section-keys to lower case we are hitting a bug.

@dvikan
Copy link
Contributor

dvikan commented Jul 23, 2022

Can confirm this bug. It happens for config keys which are case sensitive and loaded from env. Just curious, did you find any docs about this load-config-from-env feature?

@dot-mike
Copy link
Author

dot-mike commented Jul 23, 2022

Can confirm this bug. It happens for config keys which are case sensitive and loaded from env. Just curious, did you find any docs about this load-config-from-env feature?

I did not find this feature documented, but there is was a merge request implementing this feature that I learned it from: #2100

@dot-mike
Copy link
Author

dot-mike commented Jul 24, 2022

@dvikan I see your commit... but this a workaround not a fix. The coding style says classnames should PascalCase : https://rss-bridge.github.io/rss-bridge/For_Developers/Coding_style_policy.html

loadConfiguration uses the call get_class that will return classname so inevitably we will hit this bug in the future as well.

$configurationOption = Configuration::getConfig(get_class($this), $optionName);

@dvikan
Copy link
Contributor

dvikan commented Jul 24, 2022

@dot-mike I did realize that and changed that particular line. Please inspect the entire pr. I agree with your conclusion that it's not a proper fix because I only added an exception for TwitterV2 but other classes can get configs from env e.g. MemcachedCache and MastodonBridge.

dvikan added a commit to dvikan/rss-bridge that referenced this issue Jul 31, 2022
dvikan added a commit to dvikan/rss-bridge that referenced this issue Aug 19, 2022
dvikan added a commit that referenced this issue Aug 23, 2022
* refactor

* fix: case-sensitive config from env, fix #2935

* lowercase all config section and keys

* test: add test for case-insensitivity
@dvikan
Copy link
Contributor

dvikan commented Aug 23, 2022

Fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug-Report Confirmed bug report
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants