dotCMS / core

Headless/Hybrid Content Management System for Enterprises
http://dotcms.com
Other
848 stars 466 forks source link

IPUtils Test failure from incorrect state of IPUtils.disabledIpPrivateSubnet #23971

Open spbolton opened 1 year ago

spbolton commented 1 year ago

Problem Statement

A recent test CubeJSClientTest.http404 introduced in https://github.com/dotCMS/core/commit/aa04d269e272e646a9c34c11d21210f346042845 sets the flag disabledIpPrivateSubnet(true) the test does not reset this flag in a finally block to false. Depending upon the order of the unit tests being run if this test is run before IPUtilsTest.test_ip_private_subnets. it will cause its call to IPUtils.isIpPrivateSubnet to always return false and fail the Assert. Also although it may have unintended side effects anyway if multiple threads were to try to change the disabledIpPrivateSubnet static boolean in IPUtils it may be possible that One thread sets this value and during the test another thread tries to read this flag. Without it being threadsafe and volatile it may not read the correct value. To ensure a consistent setting of this value an AtomicBoolean should be used.

Steps to Reproduce

Run the unit tests and either force or rerun until CubeJSClientTest.http404 is run before IPUtilsTest.test_ip_private_subnets. Even when running before it may not fail if the tests end up in a different jvm process due to forking and gradle's daemon process.

Acceptance Criteria

IPUtilsTest.test_ip_private_subnets test passes consistently over multiple test runs

dotCMS Version

master

Proposed Objective

Quality Assurance

Proposed Priority

Priority 2 - Important

External Links... Slack Conversations, Support Tickets, Figma Designs, etc.

https://raw.githack.com/dotCMS/test-results/issue-23436-block-editor-create-video-block-node-and-insert-from-dotCMS-content-mt_6a36c6c0/projects/core/unit/reports/html/index.html

https://raw.githack.com/dotCMS/test-results/23960-remove-myspell-fix-ant-classloading_3143933d/projects/core/unit/reports/html/index.html

Assumptions & Initiation Needs

Fixed code is ready and will be added

Sub-Tasks & Estimates

No response

john-thomas-dotcms commented 6 days ago

@spbolton this was filed a long time ago. Is the problem still happening?