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.

Sign up for GitHub

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

Jump to bottom

After #39120 , Dashboard subscription's Japanese filename of attachment is broken #41669

Closed
ryoon opened this issue Apr 20, 2024 · 6 comments · Fixed by #42475
Closed

After #39120 , Dashboard subscription's Japanese filename of attachment is broken #41669

ryoon opened this issue Apr 20, 2024 · 6 comments · Fixed by #42475
Assignees
@adam-james-v
Labels
Priority:P1 Security holes w/o exploit, crashing, setup/upgrade, login, broken common features, correctness .Regression Bugs that were previously fixed and/or bugs unintentionally shipped with new features. Reporting/Export .Team/DashViz Dashboard and Viz team Type:Bug Product defects
Milestone
0.49.11

Comments

@ryoon
Copy link

ryoon commented Apr 20, 2024

Describe the bug

After #39120 , the multibyte filename of Microsoft Excel xlsx attachment of subscription is percent encoded.
Before the issue, the filename is correctly encoded and I can easily identify and open the attached file.

And percent encoded filename of the attachment may become too long to open with Microsoft Excel.

To Reproduce

  1. Create question with a name in Japanese characters, for example "テストSQL質問"
  2. Assign the question to a dashboard.
  3. Add email subscription with attached result xlsx file.
  4. Open the email with your mail user agent, for example Gmail web UI, MIcrosoft Outook web UI, Mozilla Thunderbird 115 and Microsoft Outlook included in Microsoft 365 2203.

Expected behavior

Proper filename in readable Japanese characters

Logs

No response

Information about your Metabase installation

{
  "browser-info": {
    "language": "en-US",
    "platform": "NetBSD x86_64",
    "userAgent": "Mozilla/5.0 (X11; NetBSD x86_64; rv:125.0) Gecko/20100101 Firefox/125.0",
    "vendor": ""
  },
  "system-info": {
    "file.encoding": "UTF-8",
    "java.runtime.name": "OpenJDK Runtime Environment",
    "java.runtime.version": "21.0.2-internal-adhoc.pkgsrc.jdk21u-jdk-21.0.2-13-1",
    "java.vendor": "N/A",
    "java.vendor.url": "https://openjdk.org/",
    "java.version": "21.0.2-internal",
    "java.vm.name": "OpenJDK 64-Bit Server VM",
    "java.vm.version": "21.0.2-internal-adhoc.pkgsrc.jdk21u-jdk-21.0.2-13-1",
    "os.name": "NetBSD",
    "os.version": "10.99.10",
    "user.language": "en",
    "user.timezone": "Asia/Tokyo"
  },
  "metabase-info": {
    "databases": [
      "h2"
    ],
    "hosting-env": "unknown",
    "application-database": "h2",
    "application-database-details": {
      "database": {
        "name": "H2",
        "version": "2.1.214 (2022-06-13)"
      },
      "jdbc-driver": {
        "name": "H2 JDBC Driver",
        "version": "2.1.214 (2022-06-13)"
      }
    },
    "run-mode": "prod",
    "version": {
      "date": "2024-04-16",
      "tag": "v0.49.6",
      "hash": "5abf130"
    },
    "settings": {
      "report-timezone": null
    }
  }
}

Severity

This blocks my update

Additional context

My workaround is here.

diff --git a/src/metabase/email/messages.clj b/src/metabase/email/messages.clj
index 070c231588..178d94a6cd 100644
--- a/src/metabase/email/messages.clj
+++ b/src/metabase/email/messages.clj
@@ -380,7 +380,7 @@
     {:type         :attachment
      :content-type content-type
      :file-name    (format "%s_%s.%s"
-                           (or (u/slugify card-name) "query_result")
+                           card-name
                            (u.date/format (t/zoned-date-time))
                            (name export-type))
      :content      (-> attachment-file .toURI .toURL)
@ryoon ryoon added .Needs Triage Type:Bug Product defects labels Apr 20, 2024
@paoliniluis paoliniluis added .Regression Bugs that were previously fixed and/or bugs unintentionally shipped with new features. Priority:P1 Security holes w/o exploit, crashing, setup/upgrade, login, broken common features, correctness Reporting/Export and removed .Needs Triage labels Apr 22, 2024
@paoliniluis
Copy link
Contributor

@ryoon if you have a solution please open a PR so the team can review it

@cdeweyx cdeweyx added the .Team/DashViz Dashboard and Viz team label Apr 22, 2024
@ryoon
Copy link
Author

ryoon commented Apr 29, 2024

I will try to submit my changes as a PR.

@ojjj13
Copy link

ojjj13 commented May 9, 2024 •
edited

Same happens to Chinese characters in Alerts. I am afraid it also affect other non-English languages.

@adam-james-v adam-james-v self-assigned this May 9, 2024
@adam-james-v adam-james-v mentioned this issue May 9, 2024
Merged
@adam-james-v
Copy link
Contributor

@ryoon , no need to create your own PR. I've looked at your fix and have a PR up that incorporates it, while still keeping a default filename (the 'query_result' part of the line you've changed).

Thank you for submitting this issue and your workaround! We'll work to get a fix in :)

@ryoon
Copy link
Author

ryoon commented May 10, 2024

@adam-james-v Thank you. I do not understand tests for Metabase yet and I could not send a pull request.

@adam-james-v adam-james-v added this to the 0.49.11 milestone May 13, 2024
@ryoon
Copy link
Author

ryoon commented May 14, 2024

@adam-james-v Thank you very much. I will update my Metabase instance to upcoming 0.49.11.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@adam-james-v adam-james-v

Labels
Priority:P1 Security holes w/o exploit, crashing, setup/upgrade, login, broken common features, correctness .Regression Bugs that were previously fixed and/or bugs unintentionally shipped with new features. Reporting/Export .Team/DashViz Dashboard and Viz team Type:Bug Product defects
Projects
None yet
Milestone
0.49.11
Development

Successfully merging a pull request may close this issue.

Don't use the slugify util for filenames metabase/metabase
5 participants
@ryoon @paoliniluis @cdeweyx @adam-james-v @ojjj13