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

Fixed accents filename parsing on parseFtpEntries, but you may can also patch jsftp, up to you #121

Closed
jmzoda opened this issue Mar 16, 2015 · 10 comments

Comments

@jmzoda
Copy link

jmzoda commented Mar 16, 2015

Hello,
Should be fixed on parseFtpEntries but see below for jsftp.js patch. Works like a charm using utf8 : https://github.com/mathiasbynens/utf8.js

Ftp.prototype.ls = function(filePath, callback) {
  function entriesToList(err, entries) {
    if (err) return callback(err);
    //ListingParser.parseFtpEntries(entries.text || entries, callback);
    ListingParser.parseFtpEntries(entries.text || entries, function(err, files){
      if (err) return callback(err);

      files.forEach(function(file) {
        file.name = utf8.decode(file.name);
      });
      callback(null, files);
  });
}

as usual install it with: npm install utf8 --save
and put at the beginning jsftp.js file: var utf8 = require('utf8');

Update parser.js with the utf8 decoding file names, really useful for names with accent and special characters.

Thank you

@sergi
Copy link
Owner

sergi commented Mar 19, 2015

Hi there, can you make a Pull Request and add some tests? Thanks!

@sergi sergi closed this as completed Mar 19, 2015
@sergi sergi reopened this Mar 19, 2015
@jmzoda jmzoda closed this as completed Apr 3, 2015
@jmzoda
Copy link
Author

jmzoda commented Apr 9, 2015

Hi,
As requested, I've made a pull request but only with test code included at the end of "jsftp_test.js".
The problem is quite easy to reproduce. Just copy past the code below at the end of : "jsftp_test.js"

  it("navigation through a directory containing special characters", function(next) {
    var dirName = "_éàèùâêûô_"; //  and it works with "_aaa_";
    var newDir = remoteCWD + "/" + dirName;
    ftp.raw.mkd(newDir, function(err, res) {
      assert.ok(!err);
      assert.equal(res.code, 257);
      ftp.ls(remoteCWD, function(err, res) {
        assert.ok(!err);
        assert.ok(res.some(function(el){return el.name.indexOf(dirName)>-1;}));
        ftp.raw.rmd(newDir, function(err, res) {
          assert.ok(!err);
          next();
        });
      });
    });
  });

code available on :
https://github.com/jmzoda/jsftp/blob/master/test/jsftp_test.js

My solution is already proposed on top of this issue.
Don't hesitate...

Note : I have also found minor changes because of unrecognised STAT command and stability issues on FileZilla server. Just added features check before sending STAT command.

@sergi
Copy link
Owner

sergi commented Apr 22, 2015

Hi @jmzoda. Your code doesn't pass the test, it converts "_éàèùâêûô_" into "_eaeuaeuo_", actually. Check out my changes in ebdca5b

@jmzoda
Copy link
Author

jmzoda commented Apr 22, 2015

Hi @sergi OK. I'll take a look.

@jmzoda
Copy link
Author

jmzoda commented Apr 22, 2015

Hi @sergi
I maintain that my code fixes the problem of accents on filename.
I forced the push on master of : https://github.com/jmzoda/jsftp
Please, take a look at my comments :

test/jsftp_test.js -> added check for server.stop, server is undefined when I launch the tests
lib/jsftp.js => code fixed using utf8 (code from sergi is commented)
//ListingParser.parseFtpEntries(entries.text || entries, callback);
check my code commenting it and uncomment sergi's above line on
Ftp.prototype.ls = function(filePath, callback) {...}

To test :
git clone the jmzoda/jsftp project, comment my code, put yours back and it will fail.

Note : test included, check at the end of jsftp_test.js

Hope it helps.
Never hesitate.
JMZODA

@sergi
Copy link
Owner

sergi commented May 14, 2015

Hi @jmzoda,

I cloned your branch and ran your tests. It fails on 0.12 and in iojs:

  40 passing (19s)
  1 pending
  1 failing

  1) jsftp test suite creating and listing a directory containing special characters:
     Uncaught AssertionError: false == true
      at /Users/sergi/programming/jsftp_test/test/jsftp_test.js:830:16

How are you running the tests? It clearly fails for me. Perhaps it would be good to run them in Travis.

@jmzoda
Copy link
Author

jmzoda commented May 16, 2015

@sergi

  1. Configured filezilla server to point on jsftp folder on port 3334.
  2. Launched test using : npm test > console_log.txt.
    ... as you can see at the end of tests, code works.
  3. Don't hesitate if you need more details.

jsftp@1.5.1 test D:\MyProjects\SVN.BETV.V2\BETV.DEPLOY.WEBMEDIA.EMAILING.JSFTP-FIXED\jsftp
mocha -R spec -t 5000

�[0m�[0m
Caught exception: Error: spawn mkdir ENOENT
�[0m jsftp test suite�[0m

�[32m √�[0m�[90m test initialize bad host �[0m�[31m(2607ms)�[0m

�[32m √�[0m�[90m test initialize �[0m

�[32m √�[0m�[90m test parseResponse with mark �[0m

�[32m √�[0m�[90m test parseResponse with no mark �[0m

�[32m √�[0m�[90m test send function �[0m

�[32m √�[0m�[90m test parseResponse with ignore code �[0m

�[32m √�[0m�[90m test invalid username �[0m

�[32m √�[0m�[90m test invalid password �[0m

�[32m √�[0m�[90m test getFeatures �[0m

�[32m √�[0m�[90m test print working directory �[0m
Caught exception: AssertionError: Error: 550 CWD failed. "/test/test_c9": directory not found.

�[31m 1) test switch CWD�[0m

�[32m √�[0m�[90m test switch to unexistent CWD �[0m
Caught exception: AssertionError: Error: 550 Directory not found.

�[31m 2) test passive listing of current directory�[0m

�[32m √�[0m�[90m test passive listing of nonexisting directory �[0m
Caught exception: AssertionError: [object Object]

�[31m 3) test ftp node stat�[0m

�[32m √�[0m�[90m test create and delete a directory �[0m

�[32m √�[0m�[90m test create and delete a directory containing a space �[0m

�[32m √�[0m�[90m test create and delete a file �[0m�[31m(137ms)�[0m

�[32m √�[0m�[90m test save a remote copy of a local file �[0m�[31m(248ms)�[0m

�[32m √�[0m�[90m test streaming put �[0m�[31m(207ms)�[0m

�[32m √�[0m�[90m test rename a file �[0m�[31m(227ms)�[0m
Caught exception: AssertionError: Error: 550 File not found

�[31m 4) test get a file�[0m
Caught exception: AssertionError: Error: 550 File not found

�[31m 5) test get a file and save it locally�[0m
Caught exception: AssertionError: Error: 550 File not found

�[31m 6) test get a big file stream�[0m

�[32m √�[0m�[90m test put a big file stream �[0m�[31m(443ms)�[0m

�[32m √�[0m�[90m test put a big file stream fail �[0m
Caught exception: AssertionError: false == true

�[31m 7) test get fileList array�[0m

�[32m √�[0m�[90m test reconnect �[0m

�[32m √�[0m�[90m test attach event handlers: connect �[0m
Caught exception: AssertionError: Error: 550 File not found

�[31m 8) test PASV streaming: Copy file using piping�[0m
Caught exception: AssertionError: false == true

�[31m 9) Test that streaming GET (RETR) retrieves a file properly�[0m

�[32m √�[0m�[90m Test that streaming GET (RETR) fails when a file is not present �[0m

�[32m √�[0m�[90m Test that streaming PUT (STOR) stores a file properly �[0m

�[32m √�[0m�[90m Test that streaming PUT (STOR) fails when a file is not present �[0m

�[32m √�[0m�[90m Test that onConnect is called �[0m
Caught exception: AssertionError: false == true

�[31m 10) Test for correct data on ls 1�[0m

�[32m √�[0m�[90m Test raw method with PWD �[0m

�[32m √�[0m�[90m Test raw method with HELP �[0m

�[32m √�[0m�[90m Test keep-alive with NOOP �[0m�[31m(5010ms)�[0m
�[36m - Test handling error on simultaneous PASV requests {#90}�[0m

�[32m √�[0m�[90m test set binary type �[0m
[ { name: 'testfile.txt.bak',
type: 0,
time: 1431792300000,
size: '4',
owner: 'ftp',
group: 'ftp',
userPermissions: { read: true, write: true, exec: false },
groupPermissions: { read: true, write: false, exec: false },
otherPermissions: { read: true, write: false, exec: false } },
{ name: 'éàèùâêûô',
type: 1,
time: 1431792300000,
size: '0',
owner: 'ftp',
group: 'ftp',
userPermissions: { read: true, write: true, exec: true },
groupPermissions: { read: true, write: false, exec: true },
otherPermissions: { read: true, write: false, exec: true } } ]

�[32m √�[0m�[90m creating and listing a directory containing special characters �[0m

�[92m �[0m�[32m 31 passing�[0m�[90m (13s)�[0m
�[36m �[0m�[36m 1 pending�[0m

@sergi sergi closed this as completed in b5d1a8f May 16, 2015
@jmzoda
Copy link
Author

jmzoda commented May 20, 2015

Thank you. I will check.

@jmzoda
Copy link
Author

jmzoda commented May 20, 2015

It works. Thank you. See below for an extract of the npm test result :
...
V test set binary type
V test listing a folder containing special UTF characters

37 passing (7s)
1 pending
4 failing

@princevora
Copy link

Hi @jmzoda. Your code doesn't pass the test, it converts "_éàèùâêûô_" into "_eaeuaeuo_", actually. Check out my changes in ebdca5b

can you solve my issue?

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

3 participants