Repository: pydicom/pynetdicom
Pull request #395 (2019-08-07 09:49:06)

[MRG] Improve robustness when getting local address
#### Reference issue
Related to #394 

#### Tasks
 - [x] Unit tests added that reproduce issue or prove feature is working
 - [x] Fix or feature added
 - [ ] Unit tests passing and coverage at 100% after adding fix/feature ? codecov is down, should be good



Patch:
diff --git a/docs/changelog/v1.5.0.rst b/docs/changelog/v1.5.0.rst
index fef9b67f6..af9e59ae7 100644
--- a/docs/changelog/v1.5.0.rst
+++ b/docs/changelog/v1.5.0.rst
@@ -7,6 +7,7 @@ Fixes
 .....
 
 * Fixed the AssociationServer not properly closing its sockets (:issue:`382`)
+* Improved the robustness of determining the local IP address (:issue:`394`)
 
 Changes
 .......
diff --git a/pynetdicom/ae.py b/pynetdicom/ae.py
index ab6424436..fb514ce22 100644
--- a/pynetdicom/ae.py
+++ b/pynetdicom/ae.py
@@ -510,7 +510,7 @@ class ApplicationEntity(object):
         assoc.acceptor.port = port
 
         # Association Requestor object -> local AE
-        assoc.requestor.address = socket.gethostbyname(socket.gethostname())
+        assoc.requestor.address = sock.get_local_addr()
         assoc.requestor.port = bind_address[1]
         assoc.requestor.ae_title = self.ae_title
         assoc.requestor.maximum_length = max_pdu
diff --git a/pynetdicom/dimse_messages.py b/pynetdicom/dimse_messages.py
index d38bb1672..ce47591b3 100644
--- a/pynetdicom/dimse_messages.py
+++ b/pynetdicom/dimse_messages.py
@@ -523,8 +523,11 @@ class DIMSEMessage(object):
         #   from the Message's Command Set elements
         for elem in self.command_set:
             if hasattr(primitive, elem.keyword):
-                setattr(primitive, elem.keyword,
-                        self.command_set.__getattr__(elem.keyword))
+                setattr(
+                    primitive,
+                    elem.keyword,
+                    self.command_set.__getattr__(elem.keyword)
+                )
 
         # Datasets
         # Set the primitive's DataSet/Identifier/etc attribute
diff --git a/pynetdicom/transport.py b/pynetdicom/transport.py
index b85eeb118..4b53f88ed 100644
--- a/pynetdicom/transport.py
+++ b/pynetdicom/transport.py
@@ -213,6 +213,27 @@ class AssociationSocket(object):
         """Return the Association's event queue."""
         return self.assoc.dul.event_queue
 
+    def get_local_addr(self, host=('10.255.255.255', 1)):
+        """Return an address for the local computer as str.
+
+        Parameters
+        ----------
+        host : tuple
+            The host's (addr, port) when trying to determine the local address.
+        """
+        # Solution from https://stackoverflow.com/a/28950776
+        temp = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
+        try:
+            # We use `host` to allow unit testing
+            temp.connect(host)
+            addr = temp.getsockname()[0]
+        except:
+            addr = '127.0.0.1'
+        finally:
+            temp.close()
+
+        return addr
+
     @property
     def ready(self):
         """Return ``True`` if there is data available to be read.


Test patch:
diff --git a/pynetdicom/tests/test_dimse_msg.py b/pynetdicom/tests/test_dimse_msg.py
index be886d0af..8dd628f68 100644
--- a/pynetdicom/tests/test_dimse_msg.py
+++ b/pynetdicom/tests/test_dimse_msg.py
@@ -665,12 +665,12 @@ class TestThreadSafety(object):
         cs = msg.command_set
         ds = msg.data_set
         assert cs.CommandGroupLength is None
-        assert cs.AffectedSOPClassUID == ''
+        assert cs.AffectedSOPClassUID in ['', None] # pydicom upstream change
         assert cs.CommandField is None
         assert cs.MessageID is None
         assert cs.Priority is None
         assert cs.CommandDataSetType is None
-        assert cs.AffectedSOPInstanceUID == ''
+        assert cs.AffectedSOPInstanceUID in ['', None]
         assert cs.MoveOriginatorApplicationEntityTitle is None
         assert cs.MoveOriginatorMessageID is None
         assert ds.getvalue() == b''
@@ -707,12 +707,12 @@ class TestThreadSafety(object):
         cs = msg.command_set
         ds = msg.data_set
         assert cs.CommandGroupLength is None
-        assert cs.AffectedSOPClassUID == ''
+        assert cs.AffectedSOPClassUID in ['', None] # pydicom upstream change
         assert cs.CommandField is None
         assert cs.MessageID is None
         assert cs.Priority is None
         assert cs.CommandDataSetType is None
-        assert cs.AffectedSOPInstanceUID == ''
+        assert cs.AffectedSOPInstanceUID in ['', None]
         assert cs.MoveOriginatorApplicationEntityTitle is None
         assert cs.MoveOriginatorMessageID is None
         assert ds.getvalue() == b''
diff --git a/pynetdicom/tests/test_dimse_provider.py b/pynetdicom/tests/test_dimse_provider.py
index 271c754c4..94803d418 100644
--- a/pynetdicom/tests/test_dimse_provider.py
+++ b/pynetdicom/tests/test_dimse_provider.py
@@ -161,24 +161,36 @@ class TestDIMSEProvider(object):
         assert dimse.peek_msg() == (14, primitive)
 
     def test_invalid_message(self):
+        """Test that an invalid message kills the association."""
         class DummyDUL(object):
             def __init__(self):
                 self.event_queue = queue.Queue()
 
         dimse = DIMSEServiceProvider(DummyAssociation())
+
         p_data_tf = (
-            b"\x04\x00\x00\x00\x00\x48" # P-DATA-TF 74
-            b"\x00\x00\x00\x44\x01" # PDV Item 70
+            #     |   | length
+            b"\x04\x00\x00\x00\x00\x4E" # P-DATA-TF 78
+            b"\x00\x00\x00\x4a\x01" # PDV Item 70
             b"\x03"  # PDV: 2 -> 69
-            b"\x00\x00\x00\x00\x04\x00\x00\x00\x40\x00\x00\x00"  # 12 Command Group Length
-            b"\x00\x00\x02\x00\x12\x00\x00\x00\x31\x2e\x32\x2e\x38\x34\x30\x2e\x31\x30\x30\x30\x38\x2e\x31\x2e\x31\x00"  # 26
-            b"\x00\x00\x00\x01\x02\x00\x00\x00\x30\x00"  # 10 Command Field
-            b"\x00\x00\x10\x01\x00\x00\x00\x00"  # 10 Message ID
-            b"\x00\x00\x00\x08\x02\x00\x00\x00\x01\x01"  # 10 Command Data Set Type
+            # C-ECHO-RQ
+            # CommandGroupLen | len 4         | value 64
+            b"\x00\x00\x00\x00\x04\x00\x00\x00\x40\x00\x00\x00"  # 12
+            #  AffSOPClass    | len 18        | value
+            b"\x00\x00\x02\x00\x12\x00\x00\x00"
+            b"\x31\x2e\x32\x2e\x38\x34\x30\x2e\x31\x30\x30\x30\x38\x2e\x31\x2e"
+            b"\x31\x00"  # 26
+            # CommandField    | len 2         | 0x0030 -> C-ECHO-RQ
+            b"\x00\x00\x00\x01\x02\x00\x00\x00\x30\x00"  # 10
+            # Message ID      | len 6         | -> should be invalid
+            b"\x00\x00\x10\x01\x06\x00\x00\x00\xff\xff\xff\xff\xff\xff"  # 14
+            # CommandDSType   | len 2         | no DS
+            b"\x00\x00\x00\x08\x02\x00\x00\x00\x01\x01"  # 10
         )
         pdata = P_DATA_TF()
         pdata.decode(p_data_tf)
         pdata = pdata.to_primitive()
+        # Should send Evt19 due to invalid message
         dimse.receive_primitive(pdata)
         assert dimse.assoc.dul.event_queue.get() == 'Evt19'
 
diff --git a/pynetdicom/tests/test_transport.py b/pynetdicom/tests/test_transport.py
index 2447c9c6f..a9af92277 100644
--- a/pynetdicom/tests/test_transport.py
+++ b/pynetdicom/tests/test_transport.py
@@ -171,6 +171,22 @@ class TestAssociationSocket(object):
 
         scp.shutdown()
 
+    def test_get_local_addr(self):
+        """Test get_local_addr()."""
+        # Normal use
+        self.ae = ae = AE()
+        ae.acse_timeout = 5
+        ae.dimse_timeout = 5
+        ae.network_timeout = 5
+        ae.add_requested_context(VerificationSOPClass)
+        assoc = ae.associate('', 11113)
+        assert not assoc.is_established
+        assert isinstance(assoc.requestor.address, str)
+        # Exceptional use
+        assert not assoc.is_established
+        addr = assoc.dul.socket.get_local_addr(('', 111111))
+        assert '127.0.0.1' == addr
+
 
 @pytest.fixture
 def server_context(request):