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

Force percent-encoding of fragment #1

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ypc2e55orj
Copy link

Summary

chrome.browserAction.onClicked.addListener(listener)listenerに渡すオブジェクトのurlプロパティでは、フラグメントのパーセントエンコーディングが不十分なため、TwitterはこのようなURLをURLとして認識しません。

以下は、83eb8e3 を用いてツイートした例です。

Scala Standard Library 2.13.5 - https://t.co/tFacmq0atI https://t.co/OBe7MPNPFC](y:B):(A,B)

— ypc2e55orj (@ypc2e55orj) March 10, 2021

このpull requestでは、上記のようなURLでもTwitterがURLとして認識するように改変しています。

encodeURIComponent()のみではいくつかの文字がエンコードされないため、それらは置換しています。

Scala Standard Library 2.13.5 - https://t.co/tFacmq0atI https://t.co/mSroAGseVT

— ypc2e55orj (@ypc2e55orj) March 10, 2021

Reference

https://developer.mozilla.org/ja/docs/Web/JavaScript/Reference/Global_Objects/encodeURIComponent

@foooomio
Copy link
Owner

ありがとうございます。

URLSearchParams のエンコードだけでは不十分であることは理解しました。ただ、この問題に対する修正がこれで十分であるか確信が持てていません。もう少し調査させてください。

今のところ疑問に思っている点は以下 2 つです。

  • 修正したエンコードを適用するのがフラグメントだけで十分なのかどうか。
  • 置換する文字に過不足がないかどうか。

@foooomio
Copy link
Owner

調査していてわかってきたことですが、フラグメントを encodeURIComponent()URLSearchParams() の 2 回、double encoding をしていることが、Twitter に URL を認識させる要因になっていると思われます。encodeFragment() で行っている文字の置換は、特に必要のないようにみえます。

Twitter の URL 判定の仕様を調査されたと思うのですが、先程の疑問点 2 つについて、このように修正された理由を教えていただけないでしょうか。

@ypc2e55orj
Copy link
Author

ypc2e55orj commented Mar 12, 2021

URLSearchParams()でのエンコードはデコードされるので、URLSearchParams()に渡す前のURLがツイートになると考えて差し支えないと思います。
encodeFragment()での置換は、encodeURIComponent()で置換されない半角記号( . ! ~ * ' ( ) )が末尾にあるとTwitterがURLとして認識しないことへの対処です。
例えば、https://ypc2e55orj.github.io/playground/!#! のようなURLが挙げられます。

しかし、こちらでも更に調べ直したところ(よく調べずにPRを作成して申し訳ありません)、 https://ypc2e55orj.github.io/playground/!のようなURLでもTwitterにURLとして認識されないため、

修正したエンコードを適用するのがフラグメントだけで十分なのかどうか。

はフラグメントだけでは十分ではないようです。
( https://ypc2e55orj.github.io/playground/!.html と最後に.htmlを付けると認識されることがあります。)

また、 https://github.com/ypc2e55orj/playground/tree/rfc3986-reserved-charshttps://ypc2e55orj.github.io/playground を参照していただきたいのですが、そもそもURIにこのような予約文字を使うべきではない( #1 (comment) で例に挙げたようなURLが悪い)考えも浮かんできており、申し訳ありませんが、このPRはかなり見当違いだったかもしれません。

@foooomio
Copy link
Owner

解説ありがとうございます。

Twitter の URL 判定を調べれば調べるほど意味不明で困惑しています・・・。https://example.com/()https://example.com/(hoge は invalid なのに https://example.com/(hoge) は valid なんですよね・・・。

どの文字をパーセントエンコーディングするかについてですが、少なくとも .~ をエンコードするのはあまり嬉しくないのではと思います。たとえば以下のような例は意図しないものだと思います。

また、RFC 3986 の 2.3. Unreserved Characters でも記述されていますが、unreserved な文字はパーセントエンコーディングすべきでないということだそうです。

For consistency, percent-encoded octets in the ranges of ALPHA (%41-%5A and %61-%7A), DIGIT (%30-%39), hyphen (%2D), period (%2E), underscore (%5F), or tilde (%7E) should not be created by URI producers

とはいえ末尾に置かれた .~ は、Twitter が URL と認識してくれないので対応するとなると、末尾の文字だけエンコードすることが考えられますが、次のような URL(実在するかどうかはわかりませんが・・・)の場合もエンコードするのは末尾の文字だけでいいのか判断が難しいところです。

ところで、RFC 3986 として invalid な URL を扱うかどうかですけど、本来サイト側が適切にエンコードしておいてほしいものですが、実際問題ブラウザが問題なく扱っているので、できるだけサポートしたいなと考えてはいます。

どうあるべきなのか見当がつかず、まとまりのない文章になってすみません・・・。

@foooomio
Copy link
Owner

twitter/twitter-text の Issues を見てると数年前のバグが放置されていたりして、RFC 3986 的に valid かどうかにかかわらず、このライブラリのバグった仕様に合わせる必要があるようです・・・。

怪しい文字はすべてエンコードしてしまえば URL として認識してくれるのですが、次のような URL もエンコードすべきなのかというところで頭を悩ませています。

この例ではエンコードしなくても URL として認識してくれますし、むしろエンコードしてしまうと Twitter Card が表示されなくなってしまうようです。

とはいえ、次のような URL ではリンクが壊れてしまうので、対策する必要があります。

カッコの中に文字が入っていると問題ないんですけどね・・・。

twitter/twitter-text の仕様に合わせて細かくチューニングする必要があるなあと考えている次第です。対応にお時間いただきたいです・・・。

@ypc2e55orj
Copy link
Author

ypc2e55orj commented Mar 21, 2021

これまでにいくつか考えていたことがありましたが、まず twitter/twitter-text を読んで仕様を探るべきだと思っていたので、申し訳ありませんが返信しておりませんでした。

考えていたことついて、ここに記させていただきます。

https://regex101.com/r/gUra89/1

// extended encodeURI that returns a value recognized as URL by Twitter
const extendedEncodeURI = uri => encodeURI(uri).replace(/\((?!.+\))|(?<!\(.+)\)|(?<=\)|.)[;,?:@&$.!~*'](?=$|\()/g, c => '%' + c.charCodeAt(0).toString(16).toUpperCase())

これにより、

のような動作をさせることが可能でした。ただ、正規表現部分を少しでも簡潔に書けないかと考えています。

これが twitter/twitter-text やTwitterのURL認識の仕様に合致しているかは未確認なので、漏れがあるかもしれません。

@foooomio
Copy link
Owner

foooomio commented Mar 21, 2021

先読み・後読みで表現できるのですね。これはすごいです。

MDN によると後読みに対応したのが、Chrome は 62、Firefox は 78 のようなので(先読みはわかりませんでした)、ごくわずかですが切り捨てられるユーザーが出てきますが仕方ないでしょう(アドオンの統計情報を見ると未だに Firefox 56 などを使ってるユーザーがいたりします・・・Firefox Quantum が 57 でしたっけ?)。

ところでひとつ質問なのですが、listener に渡された URL をデコード→エンコード→置換していますが、デコード→エンコードはやっておいたほうが良いのでしょうか?

@foooomio
Copy link
Owner

foooomio commented Mar 21, 2021

エンコードした URL が twitter-text に認識されるかのテストコードを書いてるんですが・・・。

const twitter = require('twitter-text')

const url = 'https://example.com/!'

const text = twitter.autoLink(url)
const valid = twitter.isValidUrl(url)

console.log({ text, valid })
// {
//   text: '<a href="https://example.com/" rel="nofollow">https://example.com/</a>!',
//   valid: true
// }

autoLink() では ! は URL から外されてしまいますけど、isValidUrl()! を含めて true で頭抱えてます(https://example.com/あいうえお なんかは false になる)。

URL 単体だと問題なくて、テキストと組み合わさったときに変な挙動になるようですね・・・。判定には extractUrls() が使えそうなのでそっち使います。

@ypc2e55orj
Copy link
Author

ypc2e55orj commented Mar 22, 2021

listenerに渡されたurlのデコード→エンコードはしたほうが良いか

これはするべきだと私は考えています。

https://www.scala-lang.org/api/current/scala/Int.html#->[B](y:B):(A,B)上でTwitter Button( 83eb8e3 )を押すと、https://www.scala-lang.org/api/current/scala/Int.html#-%3E[B](y:B):(A,B)という形でツイート内容が作成されます。これは既にtabs.Tab urlプロパティが一部パーセントエンコーディングされているからだと考えます。

twitter/twitter-text のCJKでの挙動は以下に記載がありました。

Asian languages like Chinese, Japanese or Korean may not use a delimiter such as a space to separate normal text from URLs, making it difficult to identify where the URL ends and the text starts.

For this reason Twitter-Text currently does not support extracting or auto-linking of URLs immediately followed by non-Latin characters.

Example: "http://twitter.com/は素晴らしい" . The normal text is "は素晴らしい" and is not part of the URL even though it isn't space separated.

ref: https://github.com/twitter/twitter-text/tree/master/js#urls (,が抜けていたので補ってあります。)

ところで、現在のTwitterのURL認識の仕様と twitter/twitter-text は同じなのだろうか、という疑問があります。

@foooomio
Copy link
Owner

listenerに渡されたurlのデコード→エンコードはしたほうが良いか

tabs.Tab で一部エンコードされるので、足りない分を置換して補う方法でいいのではないかと考えました。tabs.Tab によってエンコードが行われる文字の種類がわからない(Chromium や Firefox のソースコードを読めばわかるんでしょうけど)ので、不足がないか不安ではありますが、判明すれば置換リストに追加で良いかと思います。

twitter/twitter-text のCJKでの挙動

おそらくこの巻き添えを食って変な挙動になっているんでしょうね・・・。

現在のTwitterのURL認識の仕様と twitter/twitter-text は同じなのだろうか

その疑問はあったのですが、twitter/twitter-text の Description にこう書かれているので多分大丈夫かと思います。

This code is used at Twitter to tokenize and parse text to meet the expectations for what can be used on the platform.

バックエンドでは違う言語が使われている可能性もあります(Scala が使われているんでしたっけ)し、言語間による差もあるかもしれませんが、近い実装になっていると思うので問題ないと思います。

Twitter Web App で使われているか確認したかったのですが、minify されているので grep するのが難しく、断念しました・・・。

@foooomio
Copy link
Owner

foooomio commented Mar 26, 2021

テストコードを master に push しました 14a9124 。テストコードから require する都合で extendedEncodeURI() を別ファイル(src/extendedEncodeURI.js)に切り出しています。yarn test でテスト実行できます(npm でも問題ないです)。

ところで、listener に渡された url のデコード→エンコードはしたほうが良いかについて、手のひら返しになって申し訳ないですが、渡される URL がブラウザに依存してテストが書きづらいのと、Chromium と Firefox で必ずしも同じ実装になってるとは限らないので、やはりデコード→エンコードはあったほうがいいと思いました。

現状とりあえず extendedEncodeURI() は受け取った文字列をそのまま返すだけになっていますが、デコードされた文字列を受け取って適切にエンコードして返すよう実装できればと思います。疑問点などありましたら、遠慮なくおっしゃってください。

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

Successfully merging this pull request may close these issues.

2 participants