Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Miscalculates number of messages when extended character is split between messages #1

Open
Pudge601 opened this issue Mar 28, 2017 · 2 comments · May be fixed by #8
Open

Miscalculates number of messages when extended character is split between messages #1

Pudge601 opened this issue Mar 28, 2017 · 2 comments · May be fixed by #8

Comments

@Pudge601
Copy link

In the 7 bit alphabet, an extended character uses two characters (the escape character and the character itself). This pair is not allowed to be split across messages, so both the escape character and the character itself get shifted into the next message.

The current implementation doesn't handle this properly, and as a result, may miscalculate the number of messages.

An example SMS would be;

The quick brown fox jumps over the lazy dog. The quick brown fox jumps over the lazy dog. The quick brown fox jumps over the lazy dog. The quick brown f[x jumps over the lazy dog. The quick brown fox jumps over the lazy dog. The quick brown fox jumps over the lazy dog. The quick brown fox jumps over the.

The '[' at character 152 will become "0x1B[" (2 characters), and become part of the 2nd message, which pushes the last "." at the end of the content into a 3rd message, giving the following 3 messages;

The quick brown fox jumps over the lazy dog. The quick brown fox jumps over the lazy dog. The quick brown fox jumps over the lazy dog. The quick brown f

[x jumps over the lazy dog. The quick brown fox jumps over the lazy dog. The quick brown fox jumps over the lazy dog. The quick brown fox jumps over the

.

The current behaviour is that this will return 2 for the number of parts, but it should be returning 3.

@atymic
Copy link

atymic commented Mar 31, 2021

Would you accept a PR to fix this?

@chrisminett
Copy link
Member

@atymic definitely! Please include unit tests too, to guard against regressions.

@chrisminett chrisminett linked a pull request Jul 20, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants