The SMPP transport should look for PDU params to detect delivery reports #637
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -317,6 +317,25 @@ def handle_deliver_sm(self, pdu): | |||
| pdu_params = pdu['body']['mandatory_parameters'] | |||
| pdu_opts = unpacked_pdu_opts(pdu) | |||
|
|
|||
| # This might be a delivery report with PDU parameters. | |||
| receipted_message_id = pdu_opts.get('receipted_message_id', None) | |||
There was a problem hiding this comment.
Could we get a note here about the regex fallback lower down?
|
Reviewed. |
added 4 commits
October 30, 2013 16:08
|
👍 |
This was referenced Feb 9, 2019
komuw
added a commit
to komuw/naz
that referenced
this pull request
Feb 9, 2019
What:
- Fix a bug of correlation between `submit_sm_resp` and `deliver_sm`
Why:
- The way `naz` was handling correlations was:
- when sending `submit_sm` we save the `sequence_number`
- when we get `submit_sm_resp` we use its `sequence_number` and look it up from Correlater
- when we get a `deliver_sm` request, we use its `sequence_number` and look it up from Correlater
The way `naz` now does it after the fix is;
- when sending `submit_sm` we save the `sequence_number`
- when we get `submit_sm_resp` we lookup `sequence_number` and use it for correlation
- Additionally, `submit_sm_resp` has a `message_id` in the body. This is the SMSC message ID of the submitted message.
- We take this `message_id` and save it.
- when we get a `deliver_sm` request, it includes a `receipted_message_id` in the optional body parameters.
- we get that `receipted_message_id` and use it to lookup from where we had saved the one from `submit_sm_resp`
- This implementation is not without fault; as before Correlation is still on a best effort.
We are using `receipted_message_id` to correlate `submit_sm_resp` and `deliver_sm`;
However, `receipted_message_id` is an optional tag that SMSC can omit; and they do omit[1][2][3]
- fixes: #97
References:
1. opentelecoms-org/jsmpp#62
2. praekeltfoundation/vumi#298
3. praekeltfoundation/vumi#637
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The SMPP spec mentions
receipted_message_idandmessage_stateoptional params in adeliver_smPDU. Not all the SMSCs we talk to use these, but they're more likely to be authoritative and correct if they're present and we should use the content regex as a (possibly optional) fallback.