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

Whitelist of folder-list by IP address does not work on recent UMS versions #1801

Open
mxyztplk1 opened this issue May 16, 2019 · 7 comments
Open

Comments

@mxyztplk1
Copy link

@mxyztplk1 mxyztplk1 commented May 16, 2019

Whitelist of folder-list by IP address is not working for me on UMS v8.1.0. I tried UMS v7.6.2 and it also did not work. However, it works fine for me on UMS v6.7.4.

It did not work on UMS v8.1.0 on both a Windows 10 and Windows 7 system.

In an attempt to isolate or simplify the problem, I set up a version of UMS v8.1.0 with a simple configuration on a Windows 7 system to run tests. There, I specified one globally shared folder. Further, I specified the whitelist to apply to 192.168.2.91 on folder "c:\Temp2\UMSChildSafe", per manual edit of the conf file using the General Settings tab. 192.168.2.91 is the same device address for which the folder whitelist works on v6.7.4.

More specifically, the statement on the test system that apparently has no effect is:
192.168.2.91.folders = C:\Temp2\UMSTestChildsafe [with doubled backslashes]

With respect to the GUI of the device assigned to 192.168.2.91, the whitelisted folder is not presented thereon. Rather, the globally shared folder is presented (as it is for other devices on the LAN).

The logs do not show the whitelisted folder as being processed ("Checking...") on UMS v8.1.0 (and v7.6.2). The logs do not show that the whitelist statement is being rejected (or accepted, for that matter). Rather, when the device at 192.168.2.91 is connecting to UMS, it seems as though the earlier whitelist statement is not being matched with that IP address so as to use the whitelisted folder for that device, i.e., instead of the globally shared folder.

Does UMS normal release testing already include test(s) for a whitelist of a folder-list by IP address and are those test(s) working?

ums_dbg.zip

@SubJunk

This comment has been minimized.

Copy link
Member

@SubJunk SubJunk commented Aug 11, 2019

We don't have automated or manual tests for that feature. We do have automated testing for some things, maybe you could contribute some code to fix it and/or tests?

@mxyztplk1

This comment has been minimized.

Copy link
Author

@mxyztplk1 mxyztplk1 commented Aug 13, 2019

There appear to be a number of related bugs that may have the same cause/origin (i.e., code in v7.6.2 and later versions that broke sometime after v6.7.4). These bugs are as follows:

(1) whitelist of folderid_list by IP address, within a general conf file does not work (as I have documented above)
Also reported by xmp125a at: https://www.universalmediaserver.com/forum/viewtopic.php?f=3&t=13570&p=39605&hilit=uuid#p39605

(2) folders=folderid_list within a conf file for specific device(s) does not work
(in a uuid-specific conf file or in a particular renderer file):
As reported by xmp125a at: https://www.universalmediaserver.com/forum/viewtopic.php?f=3&t=13570&p=39605&hilit=uuid#p39605
I also encountered that bug in my attempt(s) to circumvent bug 1, as I said in my response to xmp125a

(3) folders_ignored does not work (see Issue #1786, ItGuillaume)

These bugs appear to involve statements that involve saving away folderid(s) for later use, either to establish the folder(s) to use for a given device when/if it connects, or to ignore folder(s) or subfolder(s) if a demanded file is located therein.

Please note that I am not referring to foldername(s), but rather folderid(s), i.e., that include a path.

Based on your last response to ItGuillaume, you may perhaps have already started to investigate the cause of bug 3. In that event, please see if that same broken code would also account for bugs 1 and 2.

As far as having an automated way to test for bug 3 in the future, a folders_ignored statement should be added to the general conf file, for some subfolder(s) of the main folder(s) under test. If an attempt to access a file in any of those subfolder(s) succeeds, the test fails.

As far a having an automated way to test for bug 1 is concerned, in a test environment that has multiple client devices, a xxx.xxx.xxx.xxx.folders = folderid_list statement should be added for one of the clients. That folderid_list should be different than the general folderid_list. If the automated tests run on that device fail to retrieve/run from the proper folderid_list, or succeed in retrieving from the wrong folderid_list, the test fails.

As far as having an automated way to test for bug 2 is concerned, in a test environment that has multiple client devices, a folders=folderid_list statement can be included in a uuid-specific conf file and/or in a specified renderer file. That folderid_list should be different than the general folderid_list. If the automated tests run on that device fail to retrieve/run from the proper folderid_list, or succeed in retrieving from the wrong folderid_list, the test fails.

@mzji

This comment has been minimized.

Copy link

@mzji mzji commented Oct 22, 2019

Seems like the whole whitelist function is removed in commit 27bcceb , so in theory the last working version with whitelist function is 7.5.0 —— @mxyztplk1 could you check this?

@mxyztplk1

This comment has been minimized.

Copy link
Author

@mxyztplk1 mxyztplk1 commented Oct 26, 2019

My limited test appears to confirm the removal of the subject Whitelist feature subsequent to UMS v7.5.0. See below for testing details.
Note that my test was limited to testing a Whitelist of a folder by IP address within a general config file (Item 1 above, but not items 2 or 3 above.)

What was the reason that the subject Whitelist function was removed subsequent to v7.5.0?


Testing details

I did a limited test of the subject Whitelist feature on UMS v7.5.0 and v7.6.0 on a Windows system. My limited test of the subject Whitelist feature succeeded on UMS v7.5.0 and failed on UMS v7.6.0.

The test for each test UMS version proceeded as follows:

  • uninstall the installed version
  • reboot the system
  • clean install the version under test
  • specify a default shared folder
  • access a file on the default shared folder from the target client device, to confirm that UMS was working generally
  • edit the UMS config file from within UMS to add an Whitelist for a different folder for the ip address of the target client device
  • exit and restart UMS
  • power down and restart the target client device
  • determine if the specified Whitelist folder was presented on the target client device; if so, access a file within that folder to further confirm that it was working
@Nadahar

This comment has been minimized.

Copy link
Contributor

@Nadahar Nadahar commented Oct 26, 2019

What was the reason that the subject Whitelist function was removed subsequent to v7.5.0?

I think I can explain that. I used to be a UMS developer, but since I felt that fixing bugs wasn't given priority (adding new features and new bugs seemed to me to be preferred), I made a "fork" called DMS where I could focus on crushing bugs without new bugs being added constantly.

The UMS devs still wanted to benefit from my code, and since both projects are open source, code can be "moved" between the two projects. The result is that a lot of DMS code has been "copied and pasted" into UMS, without much checks from what I can see. They even didn't bother to change the name many places, so you can find "DMS" all over the place in the code.

The problem however is that since the code bases aren't equal, not everything will work when just copied into the other project, and there has been quite a few problems in UMS that has been caused by this.

I suspect this is yet another example of "collateral damage" from this "lazy" copying process. I don't think it was ever intended to be removed, but too little effort was made to ensure that the result was satisfying and sound. So, I think the answer to your question is: It was an "accident".

When it comes to the reason it was removed from DMS, I don't remember the exact reason now (it was two years ago, this is the (original commit: DigitalMediaServer/DigitalMediaServer@6089bbd), but I can pretty much guarantee you that it wasn't because the whitelist functionality itself was "unwanted". Since the project has many different contributors, the code "quality" has varied a lot, and parts of the code are so badly written that I consider it less work to just remove it and write it from scratch than to try to "repair" it. I'm convinced that this was one of these cases, and I probably thought that I would just have to re-implement it in a proper way if there was ever a need. I do replace many parts of the code as I go through it, but that takes a lot of time, so some times I just figure that I will have to get back to it later if the need is really there.

@SubJunk

This comment has been minimized.

Copy link
Member

@SubJunk SubJunk commented Oct 30, 2019

I think I will just revert those changes soon so we get the previous functionality back, thanks for the investigation guys

@mxyztplk1

This comment has been minimized.

Copy link
Author

@mxyztplk1 mxyztplk1 commented Oct 31, 2019

Great! Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.