AMQP1.0 tweak and NAV CANADA message format by reidsunderland · Pull Request #1759 · MetPX/sarracenia

reidsunderland · 2026-08-18T19:16:12Z

Adjusts the AMQP1.0 connection names (NAVCAN requested that we remove MetPX from the connection name, it will like cause confusion with their version of Sundew that they refer to as MetPX)
Add NAV CANADA message format

… unique

github-actions · 2026-08-18T19:19:13Z

Test Results

360 tests 359 ✅ 1m 46s ⏱️
1 suites 1 💤
1 files 0 ❌

Results for commit 0ff06e0.

♻️ This comment has been updated with latest results.

robjarawan

Went through the AMQP1 change and exercised the new NAV CANADA format directly. The connection-name change itself looks reasonable. I left the runtime issues inline; I think those need fixing before this merges.

Tests

The full suite passes locally: 359 passed and 1 skipped. However, this PR doesn’t add any NAV CANADA tests, and the existing amq1_test.py collects zero tests. Can you add focused import/export tests covering the timestamp formats, required headers, UTF-8 and binary bodies, gzip, outbound application properties, and the new connection name?

A sanitized message fixture matching what NAV CANADA expects would be useful here. This is basically the contract for the new format and would have caught the timestamp and namespace problems.

Minor cleanup

git diff --check reports trailing whitespace in the new file, and pycodestyle finds the blank-line and indentation issues there. There’s also a print() in the gzip path that should use the logger, and its error message currently says it failed to read a SWIM message.

Since post_format navcanada is now selectable, could you also add a short config example showing the required post_topicPrefix and related message fields?

robjarawan · 2026-08-27T04:13:03Z

+                return dt.strftime("%Y%m%dT%H%M%S.%f")[:-3]
+            else:
+                try:
+                    dt = datetime.fromtimestamp(int(mt), tz=timezone.utc)


I tried both timestamp forms listed in the docstring. This uses mt, which isn’t defined in this function. A numeric string gets caught and replaced with the current time, while an integer raises NameError. That means the sample NCFILESHARE_FILE_MTIME loses its actual timestamp.

Can we parse the value passed into the function and add tests for the numeric string, integer, and ISO forms? This one needs fixing before merge.

That's a good catch, I changed mt to time

robjarawan · 2026-08-27T04:13:03Z

+        # name to work with (mirror) when writing the data from the message.
+        if 'DESTINATION' in headers:
+            msg['relPath'] += headers['DESTINATION']
+        if msg['relPath'][-1] != '/':


DESTINATION is treated as optional just above, but when it’s missing, relPath is still empty and this index raises IndexError. Since mine() claims any recognized MSG_TYPE, this becomes an AMQP decode failure and the receiver acknowledges the message as bad.

Can we either validate DESTINATION as required before decoding, or safely build a filename-only path when it’s absent? A missing-header test would catch this.

This should be fixed now

robjarawan · 2026-08-27T04:13:03Z

+                #                 if ((msg['size'] > 0) and len(data) != msg['size']):
+                #                 KeyError: 'size'
+                # inline data download does not work when size is not set
+                msg['size'] = len(decoded_payload)


This needs to store the byte length, not the number of decoded characters. I tried é: the incoming body is two bytes, but this records a size of one, so write_inline_file() rejects it after encoding it again. A non-UTF-8 body is currently returned without any content at all.

Could we preserve the decompressed bytes, use base64 for non-UTF-8 content like Message.putContentInline() does, and calculate size from those bytes? If NAV CANADA guarantees UTF-8 only, we should validate and test that explicitly.

One more edge from the adversarial pass: body=b'' skips content entirely because of if body, leaving baseUrl=none:// without content or size. A valid zero-byte file will then take the missing-download path. Please cover an empty payload explicitly too.

robjarawan · 2026-08-27T04:13:03Z

+                try:
+                    schema_ns = raw_body[ns_start:].split('"')[1]
+                    schema_ver = schema_ns.split('/')[-1]
+                    headers['MSG_NAMESPACE'] = schema_ns


The NAV CANADA input example in this file uses MSG_SCHEMA_NAMESPACE, but the exporter sends MSG_NAMESPACE. Which application-property name does NAV CANADA expect from us? Can we align this with the agreed format and lock it down with an outbound fixture?

robjarawan · 2026-08-27T04:34:59Z

+        # build relPath from DESTINATION and NCFILESHARE_FILE_NAME, so we can at least have a file path and
+        # name to work with (mirror) when writing the data from the message.
+        if 'DESTINATION' in headers:
+            msg['relPath'] += headers['DESTINATION']


I traced this through the real Flow.filter() path. With mirror True and DESTINATION ../../escape/, SR3 builds a target outside the configured directory. Since DESTINATION comes from the AMQP message, can we reject . and .. path components and verify the final path stays under the configured root? I would add a containment test for this one too.

robjarawan · 2026-08-27T04:34:59Z

+        headers['UUID'] = str(uuid.uuid4())
+
+        if 'content' in sr3_msg and sr3_msg['content']:
+            raw_body = sr3_msg['content']['value']


I tried the normal SR3 representation for binary inline content: {'encoding': 'base64', 'value': '/wA='} represents b'\xff\x00'. exportMine() sends the literal string '/wA=' instead of the original bytes. Can we decode the SR3 content using getContent() or the equivalent before posting it? I would also fail the post when content is missing instead of warning and publishing an empty body, since the NAV CAN message body is the file.

robjarawan · 2026-08-27T04:34:59Z

+                # content_encoding is only mandatory when compression is used
+                if 'amqp1_content_encoding' in headers and headers['amqp1_content_encoding'] == "gzip":
+                    if payload[:2] == b'\x1f\x8b':  # GZIP magic number
+                        decompressed_payload = gzip.decompress(payload)


I tried a highly compressible payload here. 4,892 compressed bytes became 5,000,000 bytes before SR3 got to fileSizeMax or filtering. Basically a small broker message can make the worker allocate a much larger buffer. Can we decompress with a hard expanded-size limit and reject anything over it? The 30 MB NAV CAN limit looks like the natural ceiling if that is the actual contract.

robjarawan

I went through the two new commits. The timestamp forms work now, and a missing DESTINATION safely falls back to the filename. The UTF-8 size is also using the byte count now. I reran the full suite: 359 passed and 1 skipped.

I’m still able to reproduce the remaining inline points on 0ff06e02: path traversal through DESTINATION, dropped binary and zero-byte bodies, literal base64 on export, and unbounded gzip expansion. The schema property also still needs confirmation, and there aren’t any NAV CANADA tests yet.

I think those still need fixing before merge. Ready for another look once they’re updated.

reidsunderland added 9 commits June 22, 2026 20:58

first attempt at navcan post format

5e2c4a3

Merge branch 'development' into navcan_postformat

89dc6c5

relatively finalized navcan post format

b7fead6

tweak connection name

193d6ce

fix missing bracket

7f1a41d

queueShare is not resolved, it just has variables

7506fc2

fix bugs

cf5d40c

fromisotime does not work on python3.6

7d43608

AMQP1.0 adds a prefix to the connection name so it doesn't need to be…

fff9093

… unique

reidsunderland requested review from andreleblanc11, petersilva and robjarawan August 18, 2026 19:16

Merge remote-tracking branch 'origin/development' into navcan_postformat

b07211b

robjarawan requested changes Aug 27, 2026

View reviewed changes

robjarawan reviewed Aug 27, 2026

View reviewed changes

robjarawan self-requested a review August 27, 2026 16:16

reidsunderland added 2 commits August 27, 2026 12:39

mt variable should have been time, fix

c235a69

fix relPath issues, setting the wrong msg['size']

0ff06e0

robjarawan requested changes Aug 28, 2026

View reviewed changes

Navigation Menu

Uh oh!

AMQP1.0 tweak and NAV CANADA message format - #1759

AMQP1.0 tweak and NAV CANADA message format#1759
reidsunderland wants to merge 12 commits into
developmentfrom
navcan_postformat

reidsunderland commented Aug 18, 2026

Uh oh!

github-actions Bot commented Aug 18, 2026 •

edited

Loading

Uh oh!

robjarawan left a comment

Uh oh!

robjarawan Aug 27, 2026

Uh oh!

reidsunderland Aug 27, 2026

Uh oh!

robjarawan Aug 27, 2026

Uh oh!

reidsunderland Aug 27, 2026

Uh oh!

robjarawan Aug 27, 2026

Uh oh!

robjarawan Aug 27, 2026

Uh oh!

robjarawan Aug 27, 2026

Uh oh!

robjarawan Aug 27, 2026

Uh oh!

robjarawan Aug 27, 2026

Uh oh!

robjarawan Aug 27, 2026

Uh oh!

robjarawan left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

reidsunderland commented Aug 18, 2026

Uh oh!

github-actions Bot commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

robjarawan left a comment

Choose a reason for hiding this comment

Tests

Minor cleanup

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robjarawan left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Aug 18, 2026 •

edited

Loading