Moodle

Missing semicolon in encoded CSV delimiter

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Won't Fix
  • Affects Version/s: 1.9.1
  • Fix Version/s: None
  • Component/s: Administration
  • Labels:
    None
  • Database:
    Any
  • Affected Branches:
    MOODLE_19_STABLE

Description

In 'lib/csvlib.class.php' the code to encode the CSV delimiter character misses the trailing semicolon:

$encdelim = '&#' . ord($delimiter);
=>
$encdelim = '&#' . ord($delimiter) . ';';

I have found the same problem in 'admin/user/user_bulk_download.php'. I am assigning this bug to Petr since he is mentioned as author within csvlib.class.php.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Robert Allerstorfer added a comment -

The attached patch (MDL-15065.patch) fixes this problem in both affected files.

Show
Robert Allerstorfer added a comment - The attached patch (MDL-15065.patch) fixes this problem in both affected files.
Hide
Permalink
Robert Allerstorfer added a comment - - edited

In addition, IMO the semicolon should not be offered as possible delimiter character at exporting, since its encoded form would also contain a semicolon. This makes it impossible to be used as delimiter if semicolons are present within the field's content, unless another character will be used as encloser (which is not desired).

Instead of the semicolon, I would vote for the pipe (|), since it's chance for being present in the content is minimal, and thus no or little characters have to be encoded, which saves bytes, time and makes things easier and prettier.

Show
Robert Allerstorfer added a comment - - edited In addition, IMO the semicolon should not be offered as possible delimiter character at exporting, since its encoded form would also contain a semicolon. This makes it impossible to be used as delimiter if semicolons are present within the field's content, unless another character will be used as encloser (which is not desired). Instead of the semicolon, I would vote for the pipe (|), since it's chance for being present in the content is minimal, and thus no or little characters have to be encoded, which saves bytes, time and makes things easier and prettier.
Hide
Permalink
Petr Škoda (skodak) added a comment -

Semicolon is a standard separation character for some locales in excel, it must be supported.
My +1 to keep this as is in 1.9 and remove the $encdelim completely in 2.0 and instead implement proper " quoting.

Show
Petr Škoda (skodak) added a comment - Semicolon is a standard separation character for some locales in excel, it must be supported. My +1 to keep this as is in 1.9 and remove the $encdelim completely in 2.0 and instead implement proper " quoting.
Hide
Permalink
Petr Škoda (skodak) added a comment -

Btw what what country are you from and what languages do you speak?

Show
Petr Škoda (skodak) added a comment - Btw what what country are you from and what languages do you speak?
Hide
Permalink
Robert Allerstorfer added a comment -

I am from your neighbour country Austria and thus speaking German as mother-tongue.

I understand that having ; at the import side should be supported, but at exporting I don't see much sense in offering it. Anyway, no problem for me.

What about adding the pipe nevertheless? It's really a very common CSV delimiter (all good spreadsheet apps support it - expect of Excel )

Show
Robert Allerstorfer added a comment - I am from your neighbour country Austria and thus speaking German as mother-tongue. I understand that having ; at the import side should be supported, but at exporting I don't see much sense in offering it. Anyway, no problem for me. What about adding the pipe nevertheless? It's really a very common CSV delimiter (all good spreadsheet apps support it - expect of Excel )
Hide
Permalink
Petr Škoda (skodak) added a comment - - edited

pipeline is fine for me

Show
Petr Škoda (skodak) added a comment - - edited pipeline is fine for me
Hide
Permalink
Robert Allerstorfer added a comment - - edited

OK, great - I've created MDL-15074 for this. If I post a patch there, could you please commit it (as long as it seems OK of course)?

Show
Robert Allerstorfer added a comment - - edited OK, great - I've created MDL-15074 for this. If I post a patch there, could you please commit it (as long as it seems OK of course)?
Hide
Permalink
Michael de Raadt added a comment -

Thanks for reporting this issue.

We have detected that this issue has been inactive for over a year has been recorded as affecting versions that are no longer supported.

If you believe that this issue is still relevant to current versions (2.1 and beyond), please comment on the issue. Issues left inactive for a further month will be closed.

Michael d;

lqjjLKA0p6

Show
Michael de Raadt added a comment - Thanks for reporting this issue. We have detected that this issue has been inactive for over a year has been recorded as affecting versions that are no longer supported. If you believe that this issue is still relevant to current versions (2.1 and beyond), please comment on the issue. Issues left inactive for a further month will be closed. Michael d; lqjjLKA0p6
Hide
Permalink
Michael de Raadt added a comment -

I'm closing this issue as it appears to have become inactive and is probably not relevant to a current supported version. If you are encountering this problem or one similar, please launch a new issue.

Show
Michael de Raadt added a comment - I'm closing this issue as it appears to have become inactive and is probably not relevant to a current supported version. If you are encountering this problem or one similar, please launch a new issue.

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.