Skip to content

Commit

Permalink
Allow OS to choose free port for DcmSCP.
Browse files Browse the repository at this point in the history 
This change updates the underlying network settings and API
accordingly.

Thanks to GitHub user "jogerh" (Joger Hansegard) for the proposed patch.
See also PR #63 at #63.
  • Loading branch information
@michaelonken
michaelonken committed Jul 12, 2022
1 parent 8d4a85a commit d86bead
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 39 deletions.
2 changes: 2 additions & 0 deletions dcmnet/include/dcmtk/dcmnet/assoc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,8 @@ struct DCMTK_DCMNET_EXPORT T_ASC_Association
/** network instance creation function (constructor)
* @param role association acceptor, requestor or both
* @param acceptorPort acceptor port for incoming connections.
* If for an acceptor 0 is provided, the OS will assign
* an unused ports on its own.
* For association requestors, zero should be passed here.
* @param timeout timeout for network operations, in seconds
* @param network T_ASC_Network will be allocated and returned in this parameter
Expand Down
5 changes: 4 additions & 1 deletion dcmnet/include/dcmtk/dcmnet/scp.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,10 @@ class DCMTK_DCMNET_EXPORT DcmSCP

/** Set SCP's TCP/IP listening port
* @param port [in] The port number to listen on. Note that usually on Unix-like systems
* only root user is permitted to open ports below 1024.
* only root user is permitted to open ports below 1024. If port number 0
* is provided, the OS will assign a free port once openListenPort()
* is being called. The port number actually used is then available via
* getPort().
*/
void setPort(const Uint16 port);

Expand Down
7 changes: 5 additions & 2 deletions dcmnet/include/dcmtk/dcmnet/scpcfg.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,10 @@ class DCMTK_DCMNET_EXPORT DcmSCPConfig

/** Set SCP's TCP/IP listening port
* @param port [in] The port number to listen on. Note that usually on Unix-like systems
* only root user is permitted to open ports below 1024.
* only root user is permitted to open ports below 1024. Port number 0
* can be used to let the OS assign an available free port. In that case,
* the class using this SCP configuration should update the actual port
* using setPort() once it is known.
*/
void setPort(const Uint16 port);

Expand Down Expand Up @@ -225,7 +228,7 @@ class DCMTK_DCMNET_EXPORT DcmSCPConfig

/* Get methods for SCP settings */

/** Returns TCP/IP port number SCP listens for new connection requests
/** Returns TCP/IP port number SCP listens for new connection requests.
* @return The port number
*/
Uint16 getPort() const;
Expand Down
8 changes: 8 additions & 0 deletions dcmnet/libsrc/dul.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2240,6 +2240,14 @@ initializeNetworkTCP(PRIVATE_NETWORKKEY ** key, void *parameter)
msg += OFStandard::getLastNetworkErrorCode().message();
return makeDcmnetCondition(DULC_TCPINITERROR, OF_error, msg.c_str());
}

/* If port 0 was specified by the client, the OS has assigned an unused port. */
if ((*key)->networkSpecific.TCP.port == 0) {
const u_short assignedPort = ntohs(server.sin_port);
(*key)->networkSpecific.TCP.port = assignedPort;
*(int *) parameter = assignedPort;
}

sockarg.l_onoff = 0;
if (setsockopt(sock, SOL_SOCKET, SO_LINGER, (char *) &sockarg, sizeof(sockarg)) < 0)
{
Expand Down
11 changes: 9 additions & 2 deletions dcmnet/libsrc/scp.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ OFCondition DcmSCP::openListenPort()
#ifndef DISABLE_PORT_PERMISSION_CHECK
#ifdef HAVE_GETEUID
// If port is privileged we must be as well.
if (m_cfg->getPort() < 1024 && geteuid() != 0)
if ( ( (m_cfg->getPort() < 1024) && m_cfg->getPort() !=0) && geteuid() != 0)
{
DCMNET_ERROR("No privileges to open this network port (" << m_cfg->getPort() << ")");
return NET_EC_InsufficientPortPrivileges;
Expand All @@ -113,13 +113,20 @@ OFCondition DcmSCP::openListenPort()
return result;

// Initialize network, i.e. create an instance of T_ASC_Network*.
cond = ASC_initializeNetwork(NET_ACCEPTOR, OFstatic_cast(int, m_cfg->getPort()), m_cfg->getACSETimeout(), &m_network);
const int port = OFstatic_cast(int, m_cfg->getPort());
cond = ASC_initializeNetwork(NET_ACCEPTOR, port, m_cfg->getACSETimeout(), &m_network);
if (cond.bad())
{
m_network = NULL;
return cond;
}

// Update config with assigned port if client requested it
if (port == 0)
{
m_cfg->setPort(OFstatic_cast(Uint16, m_network->acceptorPort));
}

if (m_cfg->transportLayerEnabled())
{
cond = ASC_setTransportLayer(m_network, m_cfg->getTransportLayer(), OFFalse /* Do not take over ownership */);
Expand Down
74 changes: 40 additions & 34 deletions dcmnet/tests/tscuscp.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@
#include "dcmtk/dcmnet/scu.h"


#include <cmath>

static OFLogger t_scuscp_logger= OFLog::getLogger("dcmtk.test.tscuscp");

/** SCP derived from DcmSCP in order to test two types of virtual methods:
Expand Down Expand Up @@ -167,11 +165,11 @@ struct TestSCP: DcmSCP, OFThread
/// Indicator whether related virtual notifier function was called
OFBool m_notify_assoc_termination_result;

/** Method called by OFThread to start SCP operation. Starts listen() loop of DcmSCP.
/** Method called by OFThread to start SCP operation. Starts acceptAssociations() loop of DcmSCP.
*/
virtual void run()
{
m_listen_result = listen();
m_listen_result = acceptAssociations();
}

};
Expand Down Expand Up @@ -199,6 +197,7 @@ void configure_scp_for_echo(DcmSCPConfig& cfg, Uint16 listenPort, T_ASC_SC_ROLE
*/
void scu_sends_echo(
const OFString& called_ae_title,
const Uint16 port,
const OFBool expect_assoc_reject = OFFalse,
const OFBool do_release = OFTrue,
const int secs_after_echo = 0,
Expand All @@ -210,7 +209,7 @@ void scu_sends_echo(
scu.setAETitle("TEST_SCU");
scu.setPeerAETitle(called_ae_title);
scu.setPeerHostName("localhost");
scu.setPeerPort(11112);
scu.setPeerPort(port);
OFList<OFString> xfers;
xfers.push_back(UID_LittleEndianImplicitTransferSyntax);
OFCondition result;
Expand Down Expand Up @@ -244,13 +243,15 @@ OFTEST_FLAGS(dcmnet_scp_stop_after_current_association, EF_Slow)
// accordingly
TestSCP scp;
DcmSCPConfig& config = scp.getConfig();
configure_scp_for_echo(config, 11112);
configure_scp_for_echo(config, 0);
config.setAETitle("STOP_AFTER_ASSOC");
config.setConnectionBlockingMode(DUL_BLOCK);
OFCHECK(scp.openListenPort().good());
const Uint16 port = config.getPort();
scp.start();

// Send ECHO, and wait to be sure SCP has time to exit
scu_sends_echo("STOP_AFTER_ASSOC");
scu_sends_echo("STOP_AFTER_ASSOC", port);
// Make sure server would have time to return
OFStandard::forceSleep(2);

Expand All @@ -264,7 +265,7 @@ OFTEST_FLAGS(dcmnet_scp_stop_after_current_association, EF_Slow)
// Tell SCP to stop after association and run SCU again
scp.clear();
scp.m_set_stop_after_assoc = OFTrue;
scu_sends_echo("STOP_AFTER_ASSOC");
scu_sends_echo("STOP_AFTER_ASSOC", port);
OFStandard::forceSleep(2);

// Check whether all test variables have the correct values
Expand All @@ -284,23 +285,15 @@ OFTEST_FLAGS(dcmnet_scp_fail_on_invalid_association_configuration, EF_Slow)
// Configure SCP but do not add any presentation contexts
TestSCP scp;
DcmSCPConfig& config = scp.getConfig();
config.setPort(11112);
config.setPort(0);
config.setAETitle("TEST_INVALID_CFG");
config.setConnectionBlockingMode(DUL_NOBLOCK);
// Ensure that if the server starts listen loop (it should not), the
// related thread returns anyway.
config.setConnectionTimeout(3);
scp.m_set_stop_after_timeout = OFTrue;
// Start server and check that it returns with the expected return value.
// In this case we did not configure any presentation contexts, so we
// expect the related error code.
scp.start();
scp.join();
OFCHECK(scp.m_listen_result == NET_EC_InvalidSCPAssociationProfile);
OFCHECK(scp.m_stop_after_assoc_result == OFFalse);
OFCHECK(scp.m_stop_after_timeout_result == OFFalse);
OFCHECK(scp.m_notify_connection_timeout_result == OFFalse);
OFCHECK(scp.m_notify_assoc_termination_result == OFFalse);

OFCHECK(scp.openListenPort() == NET_EC_InvalidSCPAssociationProfile);
}


Expand All @@ -312,16 +305,18 @@ OFTEST_FLAGS(dcmnet_scp_fail_on_disallowed_host, EF_Slow)
// Configure SCP for Verification
TestSCP scp;
DcmSCPConfig& config = scp.getConfig();
configure_scp_for_echo(config, 11112);
configure_scp_for_echo(config, 0);
config.setAETitle("REJECT_HOST");
config.setConnectionBlockingMode(DUL_BLOCK);
// Make sure SCP always returns
scp.m_set_stop_after_assoc = OFTrue;
// Tell SCP to reject calling host
scp.m_set_reject_calling_host = OFTrue;
OFCHECK(scp.openListenPort().good());
const Uint16 port = config.getPort();
scp.start();

scu_sends_echo("REJECT_HOST", OFTrue /* expect that association is being refused */);
scu_sends_echo("REJECT_HOST", port, OFTrue /* expect that association is being refused */);
OFStandard::forceSleep(2); // make sure server would have time to return

// Check whether all test variables have the correct values
Expand Down Expand Up @@ -349,10 +344,12 @@ OFTEST_FLAGS(dcmnet_scp_builtin_verification_support, EF_Slow)
config.setConnectionBlockingMode(DUL_BLOCK);
// Make sure server stops after the SCU connection
scp.m_set_stop_after_assoc = OFTrue;
OFCHECK(scp.openListenPort().good());
const Uint16 port = config.getPort();
scp.start();

// Send echo and receive response
scu_sends_echo("TEST_BUILTIN_VER");
scu_sends_echo("TEST_BUILTIN_VER", port);
// make sure server would have time to return
OFStandard::forceSleep(1);

Expand All @@ -374,14 +371,16 @@ OFTEST_FLAGS(dcmnet_scp_stop_after_timeout, EF_Slow)
// is performed, i.e. timeout occurs after that connection.
TestSCP scp;
DcmSCPConfig& config = scp.getConfig();
configure_scp_for_echo(config, 11112);
configure_scp_for_echo(config, 0);
config.setAETitle("STOP_ON_TIMEOUT");
config.setConnectionBlockingMode(DUL_NOBLOCK);
config.setConnectionTimeout(5);
scp.m_set_stop_after_timeout = OFTrue;
OFCHECK(scp.openListenPort().good());
const Uint16 port = config.getPort();
scp.start();
OFStandard::forceSleep(1);
scu_sends_echo("STOP_ON_TIMEOUT");
scu_sends_echo("STOP_ON_TIMEOUT", port);
// Wait for server to return after connection has been served and timeout
// has been reached
scp.join();
Expand All @@ -403,12 +402,14 @@ OFTEST_FLAGS(dcmnet_scp_no_stop_wo_request_noblock, EF_Slow)
// SCP to stop after timeout occurs, and start SCP
TestSCP scp;
DcmSCPConfig& config = scp.getConfig();
configure_scp_for_echo(config, 11112);
configure_scp_for_echo(config, 0);

config.setAETitle("NO_STOP_NOBLOCK");
config.setConnectionBlockingMode(DUL_NOBLOCK);
config.setConnectionTimeout(1);
scp.m_set_stop_after_timeout = OFFalse;

OFCHECK(scp.openListenPort().good());
scp.start();

// Check whether all test variables have the correct values, especially that
Expand All @@ -433,11 +434,13 @@ OFTEST_FLAGS(dcmnet_scp_no_stop_wo_request_block, EF_Slow)
// Configure SCP for Verification and to do not ask to exit for any reason
TestSCP scp;
DcmSCPConfig& config = scp.getConfig();
configure_scp_for_echo(config, 11112);
configure_scp_for_echo(config, 0);
config.setAETitle("NO_STOP_BLOCK");
config.setConnectionBlockingMode(DUL_BLOCK);
OFCHECK(scp.openListenPort().good());
const Uint16 port = config.getPort();
scp.start();
scu_sends_echo("NO_STOP_BLOCK");
scu_sends_echo("NO_STOP_BLOCK", port);

// Give some time to SCP so it could return
OFStandard::forceSleep(3);
Expand All @@ -452,7 +455,7 @@ OFTEST_FLAGS(dcmnet_scp_no_stop_wo_request_block, EF_Slow)

// Stop (already tested in former test cases), therefore, send another ECHO
scp.m_set_stop_after_assoc = OFTrue;
scu_sends_echo("NO_STOP_BLOCK");
scu_sends_echo("NO_STOP_BLOCK", port);
scp.join();
}

Expand All @@ -464,11 +467,12 @@ OFTEST_FLAGS(dcmnet_scp_no_term_notify_without_association, EF_Slow)
// Configure SCP for Verification and tell it stop after 3 seconds.
TestSCP scp;
DcmSCPConfig& config = scp.getConfig();
configure_scp_for_echo(config, 11112);
configure_scp_for_echo(config, 0);
config.setAETitle("NO_TERM_WO_ASSOC");
config.setConnectionBlockingMode(DUL_NOBLOCK);
config.setConnectionTimeout(3);
scp.m_set_stop_after_timeout = OFTrue;
OFCHECK(scp.openListenPort().good());
scp.start();
scp.join();
// Check whether all test variables have the correct values.
Expand Down Expand Up @@ -531,11 +535,12 @@ void test_role_selection(const T_ASC_SC_ROLE r_req,
// Configure SCP for Verification with the desired role, and start it
TestSCP scp;
DcmSCPConfig& config = scp.getConfig();
configure_scp_for_echo(config, 11112, r_acc);
configure_scp_for_echo(config, 0, r_acc);
config.setAETitle("ACCEPTOR");
config.setConnectionBlockingMode(DUL_NOBLOCK);
config.setConnectionTimeout(4);
config.setAlwaysAcceptDefaultRole(acceptDefaultInsteadOfSCP);
OFCHECK(scp.openListenPort().good());
scp.start();

// Ensure server is up and listening
Expand All @@ -546,7 +551,7 @@ void test_role_selection(const T_ASC_SC_ROLE r_req,
scu.setPeerAETitle("ACCEPTOR");
scu.setAETitle("REQUESTOR");
scu.setPeerHostName("localhost");
scu.setPeerPort(11112);
scu.setPeerPort(config.getPort());
OFList<OFString> ts;
ts.push_back(UID_LittleEndianImplicitTransferSyntax);
OFCHECK(scu.addPresentationContext(UID_VerificationSOPClass, ts, r_req).good());
Expand Down Expand Up @@ -657,9 +662,9 @@ struct TestSCPWithNCreateSupport : TestSCP
config.setConnectionBlockingMode(DUL_NOBLOCK);
config.setConnectionTimeout(10);
config.setHostLookupEnabled(OFFalse);
OFRandom rnd;
m_portNum = 65535 - rnd.getRND16() % 5535; // 60000-65535
configure_scp_for_echo(config, m_portNum);
configure_scp_for_echo(config, 0);
OFCHECK(openListenPort().good());
m_portNum = config.getPort();
OFList<OFString> xfers;
xfers.push_back(UID_LittleEndianImplicitTransferSyntax);
OFCHECK(getConfig().addPresentationContext(UID_ModalityPerformedProcedureStepSOPClass, xfers).good());
Expand Down Expand Up @@ -1109,4 +1114,5 @@ OFTEST(dcmnet_scu_sendNSETRequest_succeeds_and_sets_responsestatuscode_from_scp_
OFCHECK_MSG((result = fixture.mppsSCU.releaseAssociation()).good(), result.text());
}


#endif // WITH_THREADS

0 comments on commit d86bead

Please sign in to comment.