danielzzz / node-ping

a poor man's ping library (using udp scanning) for node
MIT License
336 stars 105 forks source link

Refactor window ping parser implementation to better parse the unit #80

Closed mondwan closed 4 years ago

mondwan commented 6 years ago

As #72 illustrate, there are different locale setting on units ms Window setting. We need to refactor the Window's parser to better support the locale variation. Maybe scanning with a list of possible units?

[en,fr,ja,zh] -> [ms]
[ru] -> [мс, мсек]
[es] ->[m, ms]
mondwan commented 6 years ago

Yet another issued related #87

aarhipkaa commented 5 years ago

error in encoding during output { host: '����⠬�', alive: true, output: '\r\n' + '����� ����⠬� � yahoo.com [98.137.246.7] � 32 ���⠬� ������:\r\n' + '�⢥� �� 98.137.246.7: �᫮ ����=32 �६�=236�� TTL=46\r\n' + '\r\n' + '����⨪� Ping ��� 98.137.246.7:\r\n' + ' ����⮢: ��ࠢ���� = 1, ����祭� = 1, ����ﭮ = 0\r\n' + ' (0% �����)\r\n' + '�ਡ����⥫쭮� �६� �ਥ��-��।�� � ��:\r\n' + ' �������쭮� = 236�ᥪ, ���ᨬ��쭮� = 236 �ᥪ, �।��� = 236 �ᥪ\r\n', time: null, min: 'unknown', max: 'unknown', avg: 'unknown', stddev: 'NaN', numeric_host: '' }

as a result of which an error node_modules\ping\lib\parser\win.js:56 this._times.push(parseFloat(match[1], 10)); ^ TypeError: Cannot read property '1' of null

mondwan commented 5 years ago

The parser is not tested on Korean?language.

Maybe, you can configure your nodejs such that Korean characters can be encoded in something bytecode or 0xaa.

The parser may work under this scenario

在 2019年11月23日週六 19:33,aarhipkaa notifications@github.com 寫道:

error in encoding during output { host: '����⠬�', alive: true, output: '\r\n' + '����� ����⠬� � yahoo.com [98.137.246.7] � 32 ���⠬� ������:\r\n' + '�⢥� �� 98.137.246.7: �᫮ ����=32 �६�=236�� TTL=46\r\n' + '\r\n' + '����⨪� Ping ��� 98.137.246.7:\r\n' + ' ����⮢: ��ࠢ���� = 1, ����祭� = 1, ����ﭮ = 0\r\n' + ' (0% �����)\r\n' + '�ਡ����⥫쭮� �६� �ਥ��-��।�� � ��:\r\n' + ' �������쭮� = 236�ᥪ, ���ᨬ��쭮� = 236 �ᥪ, �।��� = 236 �ᥪ\r\n', time: null, min: 'unknown', max: 'unknown', avg: 'unknown', stddev: 'NaN', numeric_host: '' }

as a result of which an error node_modules\ping\lib\parser\win.js:56 this._times.push(parseFloat(match[1], 10)); ^ TypeError: Cannot read property '1' of null

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/danielzzz/node-ping/issues/80?email_source=notifications&email_token=AA5YBI3QKDQSVSTK2X44JZLQVEIJZA5CNFSM4EQFNRUKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEE7TIXA#issuecomment-557790300, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA5YBI7W4D45RBEBTCMUC4DQVEIJZANCNFSM4EQFNRUA .

aarhipkaa commented 5 years ago

I have a Russian locale

aarhipkaa commented 5 years ago

here is the output from the standard cmd

C:>ping yahoo.com

Обмен пакетами с yahoo.com [98.137.246.7] с 32 байтами данных: Ответ от 98.137.246.7: число байт=32 время=218мс TTL=51 Ответ от 98.137.246.7: число байт=32 время=219мс TTL=51 Ответ от 98.137.246.7: число байт=32 время=217мс TTL=51 Ответ от 98.137.246.7: число байт=32 время=217мс TTL=51

Статистика Ping для 98.137.246.7: Пакетов: отправлено = 4, получено = 4, потеряно = 0 (0% потерь) Приблизительное время приема-передачи в мс: Минимальное = 217мсек, Максимальное = 219 мсек, Среднее = 217 мсек

mondwan commented 5 years ago

This language is not yet support.

It will be great if you can send us a MR for patching this.

在 2019年11月23日週六 20:55,aarhipkaa notifications@github.com 寫道:

here is the output from the standard cmd

C:>ping yahoo.com

Обмен пакетами с yahoo.com [98.137.246.7] с 32 байтами данных: Ответ от 98.137.246.7: число байт=32 время=218мс TTL=51 Ответ от 98.137.246.7: число байт=32 время=219мс TTL=51 Ответ от 98.137.246.7: число байт=32 время=217мс TTL=51 Ответ от 98.137.246.7: число байт=32 время=217мс TTL=51

Статистика Ping для 98.137.246.7: Пакетов: отправлено = 4, получено = 4, потеряно = 0 (0% потерь) Приблизительное время приема-передачи в мс: Минимальное = 217мсек, Максимальное = 219 мсек, Среднее = 217 мсек

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/danielzzz/node-ping/issues/80?email_source=notifications&email_token=AA5YBI35QZKAZAGPQERGO7TQVER5BA5CNFSM4EQFNRUKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEE7US4Y#issuecomment-557795699, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA5YBI6EWWUPUTCGI67G2XDQVER5BANCNFSM4EQFNRUA .

aarhipkaa commented 5 years ago

this seems to be an embedded terminal issue. in the wsl terminal windows 10 is working fine

host: 'yahoo.com', alive: true, output: 'PING yahoo.com (98.137.246.8) 56(84) bytes of data.\n64 bytes from 98.137.246.8: icmp_seq=1 ttl=51 time=217 ms\n\n--- yahoo.com ping statistics ---\n1 packets transmitted, 1 received, 0% packet loss, time 0ms\nrtt min/avg/max/mdev = 217.117/217.117/217.117/0.000 ms\n', time: 217, min: '217.117', max: '217.117', avg: '217.117', stddev: '0.000', numeric_host: '98.137.246.8'

mondwan commented 5 years ago

Ummm, as long as you are feeding the parser with supported language, it should be fined.

在 2019年11月23日週六 21:30,aarhipkaa notifications@github.com 寫道:

this seems to be an embedded terminal issue. in the wsl terminal windows 10 is working fine

host: 'yahoo.com', alive: true, output: 'PING yahoo.com (98.137.246.8) 56(84) bytes of data.\n64 bytes from 98.137.246.8: icmp_seq=1 ttl=51 time=217 ms\n\n--- yahoo.com ping statistics ---\n1 packets transmitted, 1 received, 0% packet loss, time 0ms\nrtt min/avg/max/mdev = 217.117/217.117/217.117/0.000 ms\n', time: 217, min: '217.117', max: '217.117', avg: '217.117', stddev: '0.000', numeric_host: '98.137.246.8'

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/danielzzz/node-ping/issues/80?email_source=notifications&email_token=AA5YBIYB4QN56HC6U3B63LDQVEV5ZA5CNFSM4EQFNRUKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEE7VF5Y#issuecomment-557798135, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA5YBI57Y3S5MRV6PTHIAPLQVEV5ZANCNFSM4EQFNRUA .

aarhipkaa commented 5 years ago

I did it!!! { host: '72.30.35.10', alive: true, output: '\r\n' + 'Обмен пакетами с 72.30.35.10 по с 32 байтами данных:\r\n' + 'Ответ от 72.30.35.10: число байт=32 время=164мс TTL=45\r\n' + 'Ответ от 72.30.35.10: число байт=32 время=162мс TTL=45\r\n' + 'Ответ от 72.30.35.10: число байт=32 время=163мс TTL=45\r\n' + 'Ответ от 72.30.35.10: число байт=32 время=162мс TTL=45\r\n' + 'Ответ от 72.30.35.10: число байт=32 время=162мс TTL=45\r\n' + 'Ответ от 72.30.35.10: число байт=32 время=162мс TTL=45\r\n' + 'Ответ от 72.30.35.10: число байт=32 время=162мс TTL=45\r\n' + 'Ответ от 72.30.35.10: число байт=32 время=162мс TTL=45\r\n' + 'Ответ от 72.30.35.10: число байт=32 время=162мс TTL=45\r\n' + 'Ответ от 72.30.35.10: число байт=32 время=162мс TTL=45\r\n' + '\r\n' + 'Статистика Ping для 72.30.35.10:\r\n' + ' Пакетов: отправлено = 10, получено = 10, потеряно = 0\r\n' + ' (0% потерь)\r\n' + 'Приблизительное время приема-передачи в мс:\r\n' + ' Минимальное = 162мсек, Максимальное = 164 мсек, Среднее = 162 мсек\r\n', time: '164', min: '162.000', max: '164.000', avg: '162.000', stddev: '2.000', numeric_host: '72.30.35.10' }

aarhipkaa commented 5 years ago

I added a few lines, but after that the parser does not work correctly

ping-promise.js var iconv = require('iconv-lite'); ... // Cache all lines from the system ping var outstring = []; ping.stdout.on('data', function (data) { if (platform === "win32") { // decoding from Buffer outstring.push(iconv.decode(data, "cp866").toString()); console.log(outstring); } else { outstring.push(String(data)); } });

aarhipkaa commented 5 years ago

Then I rewrote the parser for Win. It may not be right, but it does my job.

win.js

'use strict';

var util = require('util'); var __ = require('underscore');

var base = require('./base');

/**

util.inherits(WinParser, base);

/**

/**

/**

module.exports = WinParser;

aarhipkaa commented 5 years ago

for because of the locale "es", "line.indexOf(this.localTime[0])" should be replaced by "line.lastIndexOf(this.localTime[0])"

mondwan commented 5 years ago

Thanks for your work.

Would you mind to send us a MR so that we can review, and try to patch this library in order to support Russian locale.

在 2019年11月27日週三 12:48,aarhipkaa notifications@github.com 寫道:

for because of the locale "es", "line.indexOf(this.localTime[0])" should be replaced by let i = line.lastIndexOf(this.localTime[0])

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/danielzzz/node-ping/issues/80?email_source=notifications&email_token=AA5YBI2BGRNH4JCX3V4N2LLQVX33LA5CNFSM4EQFNRUKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFIIYWY#issuecomment-558926939, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA5YBI4TILPZCYOHKX5RZQ3QVX33LANCNFSM4EQFNRUA .

aarhipkaa commented 5 years ago

Sorry, I don’t know what MR is. I recently started to study programming, and I have never written to Github before. Sorry for my English.

mondwan commented 5 years ago

Welcome to the programming world.

MR stands for Merge request. Basically, it is nearly equivalent to asking for sending a diff file so that we can review, and apply your codes into our library. This is a common way for contribute opening source project in github (or other git repository)

You can take a look of the tutorial from github about how to work with MR

We are looking forward to see your patch regarding to your Russian locale patch :)

在 2019年11月27日週三 18:56,aarhipkaa notifications@github.com 寫道:

Sorry, I don’t know what MR is. I recently started to study programming, and I have never written to Github before. Sorry for my English.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/danielzzz/node-ping/issues/80?email_source=notifications&email_token=AA5YBI5XIV7DNEHPEUXZPATQVZG4TA5CNFSM4EQFNRUKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFJD25Y#issuecomment-559037815, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA5YBI4HGAS3W466TWZHBXLQVZG4TANCNFSM4EQFNRUA .

adron-s commented 4 years ago

Hello. Last version(0.2.3) is not work on Windows 10 and Russian UTF8 locale.

Here is my fast fix:

--- ./win.js.orig   2020-03-26 08:47:17.000000000 +0200
+++ ./win.js    2020-03-26 10:41:46.495708439 +0200
@@ -11,6 +11,8 @@
  */
 function WinParser(config) {
     base.call(this, config);
+    var locale = Intl.DateTimeFormat().resolvedOptions().locale;
+    this.is_rus = /^ru/.test(locale);
     this._ipv4Regex = /^([0-9]{1,3}\.){3}[0-9]{1,3}$/;
 }

@@ -60,6 +62,10 @@
  * @param {string} line - A line from system ping
  */
 WinParser.prototype._processBody = function (line) {
+    if(this.is_rus){
+      //console.log("!", line.split('').map(c => c.charCodeAt(0)));
+      line = line.replace(/(=\d+)\ufffd\ufffd(\s+)/, '$1ms$2');
+    }
     var tokens = line.split(' ');
     var kvps = __.filter(tokens, function (token) {
         // Sometime it shows <1ms

The problem is caused by the fact that the ping result string is not in utf8 encoding:

[
  65533, 10405, 65533,    32, 65533, 65533,
     32,    56,    46,    56,    46,    56,
     46,    56,    58,    32, 65533,  6894,
     32, 65533, 65533, 65533, 65533,    61,
     51,    50,    32, 65533,  2412, 65533,
     61,    49, 65533, 65533,    32,    84,
     84,    76,    61,    53,    56
].map(c => String.fromCharCode(c));

So using "мс|мсек" search is not suitable!

mondwan commented 4 years ago

Hi, there is a patch on #110 which try to fix this error. Please check this out on master branch. Thanks

mondwan commented 4 years ago

Reopen if there are any issues

baur commented 2 years ago

Same problem!

Windows11

image
mondwan commented 2 years ago

Hi, as the original thread, there are no MRs, and there are no fixtures supplied. So, there are no fixes yet.

Please include the corresponding fixtures as examples so that some1 can implement them

On Tue, 19 Jul 2022, 19:13 Bauyrzhan Yessetov, @.***> wrote:

Same problem!

Windows11

[image: image] https://user-images.githubusercontent.com/2898361/179819927-c9051e94-594c-4565-bc24-a332a69c6100.png

— Reply to this email directly, view it on GitHub https://github.com/danielzzz/node-ping/issues/80#issuecomment-1189403251, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA5YBI3KR5V5EM7G76CY7JDVU3V23ANCNFSM4EQFNRUA . You are receiving this because you modified the open/close state.Message ID: @.***>

baur commented 2 years ago

Please include the corresponding fixtures as examples so that some1 can implement them

Hi, thank you for your reply, I'm expected the next result:

C:\Users\admin>ping 127.0.0.01

Обмен пакетами с 127.0.0.1 по с 32 байтами данных:
Ответ от 127.0.0.1: число байт=32 время<1мс TTL=128
Ответ от 127.0.0.1: число байт=32 время<1мс TTL=128
Ответ от 127.0.0.1: число байт=32 время<1мс TTL=128
Ответ от 127.0.0.1: число байт=32 время<1мс TTL=128

Статистика Ping для 127.0.0.1:
    Пакетов: отправлено = 4, получено = 4, потеряно = 0
    (0% потерь)
Приблизительное время приема-передачи в мс:
    Минимальное = 0мсек, Максимальное = 0 мсек, Среднее = 0 мсек
mondwan commented 2 years ago

Thanks for the samples. But, we need more for supporting a new language.

Please take a look the contributing guideline. Since not every developers can read the new language, we need the requester to provide sufficient information for us to work.

https://github.com/danielzzz/node-ping/blob/master/CONTRIBUTING.md

Let me know if you have any questions

On Wed, 20 Jul 2022, 03:12 Bauyrzhan Yessetov, @.***> wrote:

Please include the corresponding fixtures as examples so that some1 can implement them

Hi, thank you for your reply, I'm expected the next result:

C:\Users\admin>ping 127.0.0.01

Обмен пакетами с 127.0.0.1 по с 32 байтами данных: Ответ от 127.0.0.1: число байт=32 время<1мс TTL=128 Ответ от 127.0.0.1: число байт=32 время<1мс TTL=128 Ответ от 127.0.0.1: число байт=32 время<1мс TTL=128 Ответ от 127.0.0.1: число байт=32 время<1мс TTL=128

Статистика Ping для 127.0.0.1: Пакетов: отправлено = 4, получено = 4, потеряно = 0 (0% потерь) Приблизительное время приема-передачи в мс: Минимальное = 0мсек, Максимальное = 0 мсек, Среднее = 0 мсек

— Reply to this email directly, view it on GitHub https://github.com/danielzzz/node-ping/issues/80#issuecomment-1189728854, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA5YBI4BNNRSL6TI6XX4AJ3VU5OB7ANCNFSM4EQFNRUA . You are receiving this because you modified the open/close state.Message ID: @.***>

baur commented 2 years ago

This is not a new language, this is Russian, same in this thread supported by node-ping

mondwan commented 2 years ago

Ah, cool. I will take a look, and see what I can do later on