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

fix(telegram): No user avatar when using local bot API. #312

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

su226
Copy link
Contributor

@su226 su226 commented Aug 21, 2024

When enable files.local only, return a data URL since browsers won't load file:// URLs.

When enable both files.local and files.server, adapter generates a URL like http://127.0.0.1:5140/telegram/your_bot_id//path_to_local_bot_api/your_bot_token/photos/file_1.jpg, which was not handled by router as intended (Although this may lead to a security breach of arbitrary file read, should there be a better way to handle this? Maybe use file_id in files route instead of file_path), so I fixed the route too. This should also fix other file-related APIs when using both files.local and files.server.

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.93%. Comparing base (e5825f1) to head (a5a2374).
Report is 117 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #312      +/-   ##
==========================================
+ Coverage   86.88%   86.93%   +0.04%     
==========================================
  Files           1        1              
  Lines         549      551       +2     
  Branches      111      111              
==========================================
+ Hits          477      479       +2     
  Misses         72       72              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@shigma
Copy link
Contributor

shigma commented Aug 22, 2024

return a data URL since browsers won't load file:// URLs

Does this have anything to do with the browser?

In fact, if the transferred media is huge, using base64 will also lead to additional problems. If there is a need to access local files in the browser, the best approach may be to provide a proxy service locally.

@su226
Copy link
Contributor Author

su226 commented Aug 23, 2024

Does this have anything to do with the browser?

I tried file:// URL and I got nothing in Koishi WebUI.

In fact, if the transferred media is huge, using base64 will also lead to additional problems. If there is a need to access local files in the browser, the best approach may be to provide a proxy service locally.

I guess it's files.server's purpose? Data URL is just for the case when files.server is disabled. <img> element in message.create events will also contain base64 if files.server is not enabled.

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

Successfully merging this pull request may close these issues.

3 participants