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

Latin-1 encoding in CSV presents invalid characters #9365

Closed
jalvesbsolus opened this issue May 10, 2024 · 14 comments
Closed

Latin-1 encoding in CSV presents invalid characters #9365

jalvesbsolus opened this issue May 10, 2024 · 14 comments
Labels

Comments

@jalvesbsolus
Copy link

Bug Description

When using the flag N8N_DEFAULT_BINARY_DATA_MODE=filesystem to read a 'csv' file with the 'raw data' option set to true and 'Read As String' set to false, characters such as accents appear invalid when use latin-1 encoding.

Attention: I specify the options used for a reason - when N8N_DEFAULT_BINARY_DATA_MODE=default works correctly, the issue arises with N8N_DEFAULT_BINARY_DATA_MODE=filesystem.

I have already tried with 'Read As String' set to true (which works for UTF-8) but it doesn't work either.

To Reproduce

  1. Change N8N_DEFAULT_BINARY_DATA_MODE=filesystem
  2. 'Read As String' set to false and 'raw data' option set to true (only Just because these options work for the flag N8N_DEFAULT_BINARY_DATA_MODE=default)
  3. import csv with latin-1
  4. See the accents

Expected behavior

  1. Use N8N_DEFAULT_BINARY_DATA_MODE=filesystem
  2. import csv in latin-1
  3. keep compatibility to utf-8 and utf-8 with bom

Finally, I use N8N_DEFAULT_BINARY_DATA_MODE=filesystem with CSV encoding latin-1, utf-8, and utf-8 with BOM (these last two already work).

Operating System

centos

n8n Version

1.39.1

Node.js Version

v18.19.1

Database

MySQL

Execution mode

own (deprecated)

@Joffcom
Copy link
Member

Joffcom commented May 14, 2024

Hey @jalvesbsolus,

Thanks for the report, I have put in a PR that should resolve this. Once reviewed and merged it will be available in a future release.

@jalvesbsolus
Copy link
Author

jalvesbsolus commented May 16, 2024

Thanks @Joffcom .
People, can anyone follow up on this? I needed this resolution

@Joffcom
Copy link
Member

Joffcom commented May 16, 2024

@jalvesbsolus when you say "people can anyone follow up on this" what do you mean?

The PR is currently with our nodes team waiting to be reviewed between tasks. Once the code has been reviewed internally I will then merge the PR and when the next release goes out it will be part of that so it could be next Wednesday when we do our regular release unless we do a bug fix release before then.

@jalvesbsolus
Copy link
Author

@Joffcom "people can anyone follow up on this" means - in this case it would be a "reviewed"

@Joffcom
Copy link
Member

Joffcom commented May 16, 2024

@jalvesbsolus Ah ok, In that case it will happen whenever someone is available to do it. I know I will pick up reviews between tasks and I suspect the rest of the team is the same.

@Joffcom
Copy link
Member

Joffcom commented May 16, 2024

@jalvesbsolus it has just been reviewed and merged so it will be in the next release.

@jalvesbsolus
Copy link
Author

@Joffcom thanks

@janober
Copy link
Member

janober commented May 22, 2024

Fix got released with n8n@1.43.0

@jalvesbsolus
Copy link
Author

jalvesbsolus commented May 22, 2024

@janober or @Joffcom problem when updated. I got this error in login:
MaxListenersExceededWarning: Possible EventEmitter memory leak detected

FULL
(node:6) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 editorUiConnected listeners added to [Push]. Use emitter.setMaxListeners() to increase limit
(Use node --trace-warnings ... to show where the warning was created)

@Joffcom
Copy link
Member

Joffcom commented May 29, 2024

Hey @jalvesbsolus,

That is likely to be unrelated to this change and possibly related to your system being overloaded at the time. Are you still seeing the issue now?

@netroy
Copy link
Member

netroy commented Jun 3, 2024

Closing this, since the fix was released 2 weeks ago.

@netroy netroy closed this as completed Jun 3, 2024
@souzagaabriel
Copy link

The Encoding option only appears when N8N_DEFAULT_BINARY_DATA_MODE=default . I don't know if this is the expected behavior.

@jalvesbsolus
Copy link
Author

jalvesbsolus commented Jul 12, 2024

@Joffcom or @janober one question. How to identify the file's encoding automatically, is there any way?

I try with node "code" and lang Pyhton
`
def detectEncoding(binary_data):

encodings = ['utf-8', 'ascii', 'utf-16', 'latin1']
for encoding in encodings:
    try:
        binary_data.decode(encoding)
        return encoding
    except (UnicodeDecodeError, LookupError):
        continue

return 'Latin1'`

This is work in N8N_DEFAULT_BINARY_DATA_MODE=default but in N8N_DEFAULT_BINARY_DATA_MODE=filesystem not

@jalvesbsolus
Copy link
Author

@souzagaabriel I don't have this problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants