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

Wrong nsn result when trying to parse incomplete number of country with leading digits #35

Closed
cupofme opened this issue Dec 7, 2022 · 10 comments

Comments

@cupofme
Copy link

cupofme commented Dec 7, 2022

When trying to format incomplete numbers for "as-you-type" formatting, some countries produce wrong numbers by adding excess leading number at some stage. I were able to reproduce it only for countries with existing nationalPrefixTransformRule in metadata.

Code to reproduce

import 'package:phone_numbers_parser/phone_numbers_parser.dart';

void main() {
  final fullBermudaNumber = PhoneNumber.parse('+14412957070');
  final incompleteBermudaNumber = PhoneNumber.parse('+14412957');

  print(fullBermudaNumber.nsn); // 4412957070 OK
  print(fullBermudaNumber.getFormattedNsn()); // 4412957070 OK

  print(incompleteBermudaNumber.nsn); // 4414412957 WRONG (4412957 expected)
  print(incompleteBermudaNumber.getFormattedNsn()); // 4414412957 WRONG (4412957 expected)

  final fullUSNumber = PhoneNumber.parse('+14056343023');
  final incompleteUSNumber = PhoneNumber.parse('+14056343');

  print(fullUSNumber.nsn); // 4056343023 OK
  print(fullUSNumber.getFormattedNsn()); // 405-634-3023 OK

  print(incompleteUSNumber.nsn); // OK
  print(incompleteUSNumber.getFormattedNsn()); // 405-634-3 OK
}
@cedvdb
Copy link
Owner

cedvdb commented Dec 7, 2022

The patterns for bermuda are here:

https://github.com/cedvdb/dart_phone_metadata/blob/main/lib/src/generated/metadata_patterns_by_iso_code.dart#L209

    nationalPrefixForParsing: r'1|([2-8]\d{6})$',
    nationalPrefixTransformRule: r'441$1',

It seems like the pattern is expecting 1 followed by 7 digits (with the first one being between 2 and 8 included) and then adding 441 instead of the 1 as can be seen on the next line.

All in all this fits the parsing rules and the problem seems to be that this block should not be called at all if it was originally a international phone number, which yours is.

Which leads me to believe this line should not be called if an exit code was found before

https://github.com/cedvdb/phone_numbers_parser/blob/dev/lib/src/parsers/phone_parser.dart#L64

    nsn = NationalNumberParser.transformLocalNsnToInternationalUsingPatterns(
      nsn,
      destinationMetadata,
    );

should be surrounded by an if statement

@cupofme
Copy link
Author

cupofme commented Dec 8, 2022

@cedvdb Thank you for the quick response! I'll probably try to implement the fix and make a PR

@cupofme cupofme changed the title Wrong nsn result when trying to parse incomplete number with non-null nationalPrefixTransformRule Wrong nsn result when trying to parse incomplete number of country with leading digits Dec 8, 2022
@cedvdb
Copy link
Owner

cedvdb commented Dec 12, 2022

Did you have a stab at it ?

@cupofme
Copy link
Author

cupofme commented Dec 13, 2022

Sorry, didn't have enough time to dive deeply into it. Tried to implement naive fix, but it feels obviously wrong and even fails for some regions (mostly British overseas territories)

@nahimilega
Copy link

Hey @cedvdb @cupofme , if possible could you provide an estimated time to fix this bug fix... because I hope you agree it's kinda a major bug..not allowing certain phone numbers.. hampering the business of all the users of the library.

@cedvdb
Copy link
Owner

cedvdb commented Jan 13, 2023

@nahimilega No timeline from me, although I might have a look next week. I don't believe the fix is that hard though. In the meantime if it's pressing to you, I'll review a PR.

@blopker
Copy link

blopker commented Feb 9, 2023

@cedvdb, first, thanks for this lib, so much better than using all the platform-specific stuff. Does the fix @cupofme made work for you? Happy to make a PR for it if so.

@cedvdb
Copy link
Owner

cedvdb commented Feb 11, 2023

No it did not work as intended, my comment here should give a rough idea of what's going on: #35 (comment) , but it has to be tested because I remember seeing something that did not work properly, by me or someone else.
A PR would very much be appreciated.

Just my rough 10 seconds analysis is that this line https://github.com/cedvdb/phone_numbers_parser/blob/dev/lib/src/parsers/phone_parser.dart#L64 should not be called when we know it's an international phone number (there was an exit code or country code). Or alternatively, if there was no national prefix I imagine. Would have to double check how this is supposed to work again for countries like Argentina which have different phone number version for international and national. We just need to know when this method should actually be called and call it accordingly.

@blopker
Copy link

blopker commented Feb 11, 2023

I see. Are there a few test cases you could provide that fail, so I know exactly what the issue is? Or is the code in the reproduction a good test?

@cedvdb
Copy link
Owner

cedvdb commented Feb 19, 2023

Fixed in version 7.0.1

@cedvdb cedvdb closed this as completed Feb 19, 2023
@cedvdb cedvdb mentioned this issue Feb 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants