search

jcorporation / myMPD

myMPD is a standalone and mobile friendly web mpd client with a tiny footprint and advanced features.
https://jcorporation.github.io/myMPD/
GNU General Public License v3.0
419 stars 65 forks source link

Documentation for ACL incorrect #663

Closed pgzh closed 2 years ago

pgzh commented 2 years ago

myMPD version: 9.0.4

The documentation examples for configuring the ACL are incorrect.

To Reproduce

Steps to reproduce the behavior:

  1. Configure ACL as described in the documentation at https://jcorporation.github.io/myMPD/configuration/acl
  2. myMPD returns HTTP 403 "myMPD error Request blocked by ACL"

Expected behavior myMPD will not block / documentation shows correct examples

Screenshots

N/A

Server plattform (please complete the following information):

Client plattform (please complete the following information):

Debug logs (please attach if it can be usefull)

N/A

Configuration (please attach if it can be usefull)

N/A

Additional context

Invalid ACL (as in documentation): +127.0.0.0/8,+10.0.7.0/24

Valid ACL: +127.0.0/8,+10.0.7/24

The Mongoose webserver will fail checking the ACLs with the last segment of the IP adress provided and only work without it. In my opinion this is plain wrong behavior of Mongoose as 127.0.0/8 and 10.0.7/24 are not valid IP networks but 127.0.0.0/8,+10.0.7.0/24 are actually correct (but will not work). The relevant function is mg_check_ip_acl() called from web_server.c by myMPD.

The myMPD documentation examples should be changed to +127.0.0/8,+192.168.0/24 for now.

This should probably be reported as a bug against Mongoose and fixed upstream there. I have not filed a bug report against Mongoose yet because I'd like to hear your opinion on this or let you report this to Mongoose directly.

jcorporation commented 2 years ago

Let me check the myMPD code and documentation of mongoose, before we opening a upstream issue.

jcorporation commented 2 years ago

Mongoose documentation: https://mongoose.ws/documentation/#mg_check_ip_acl

It seems +127.0.0.0/8,+10.0.7.0/24 should be correct, also the unit_tests uses the same notation.

I found that I compare the return value of mg_check_ip_acl with a bool value, but changing this to int as documented has no positive effect.

jcorporation commented 2 years ago

It seems to be a mongoose issue: https://github.com/cesanta/mongoose/issues/1452

pgzh commented 2 years ago

Maybe a hint on the documentation page about the incorrect handling of the addresses and networks of Mongoose would be convenient until this is fixed.

Thank you very much for looking into this and constantly improving myMPD !

jcorporation commented 2 years ago

Is now fixed in the devel branch (v9.1.0).