Invalid input for update_ip_address

Hi,

There is a lot of errors in /logs, which states there is invalid input for update_ip_address operation.

The discourse is deployed in Azure, with a separated PostgreSQL and Redis service.

Error logs:

/var/www/discourse/vendor/bundle/ruby/2.4.0/gems/rack-mini-profiler-0.10.7/lib/patches/db/pg.rb:90:in `async_exec'
/var/www/discourse/vendor/bundle/ruby/2.4.0/gems/rack-mini-profiler-0.10.7/lib/patches/db/pg.rb:90:in `async_exec'
/var/www/discourse/vendor/bundle/ruby/2.4.0/gems/activerecord-5.1.4/lib/active_record/connection_adapters/postgresql_adapter.rb:614:in `block (2 levels) in exec_no_cache'
/var/www/discourse/vendor/bundle/ruby/2.4.0/gems/activesupport-5.1.4/lib/active_support/dependencies/interlock.rb:46:in `block in permit_concurrent_loads'
/var/www/discourse/vendor/bundle/ruby/2.4.0/gems/activesupport-5.1.4/lib/active_support/concurrency/share_lock.rb:185:in `yield_shares'
/var/www/discourse/vendor/bundle/ruby/2.4.0/gems/activesupport-5.1.4/lib/active_support/dependencies/interlock.rb:45:in `permit_concurrent_loads'
/var/www/discourse/vendor/bundle/ruby/2.4.0/gems/activerecord-5.1.4/lib/active_record/connection_adapters/postgresql_adapter.rb:613:in `block in exec_no_cache'
/var/www/discourse/vendor/bundle/ruby/2.4.0/gems/activerecord-5.1.4/lib/active_record/connection_adapters/abstract_adapter.rb:612:in `block (2 levels) in log'
/usr/local/lib/ruby/2.4.0/monitor.rb:214:in `mon_synchronize'
/var/www/discourse/vendor/bundle/ruby/2.4.0/gems/activerecord-5.1.4/lib/active_record/connection_adapters/abstract_adapter.rb:611:in `block in log'
/var/www/discourse/vendor/bundle/ruby/2.4.0/gems/activesupport-5.1.4/lib/active_support/notifications/instrumenter.rb:21:in `instrument'
/var/www/discourse/vendor/bundle/ruby/2.4.0/gems/activerecord-5.1.4/lib/active_record/connection_adapters/abstract_adapter.rb:603:in `log'
/var/www/discourse/vendor/bundle/ruby/2.4.0/gems/activerecord-5.1.4/lib/active_record/connection_adapters/postgresql_adapter.rb:612:in `exec_no_cache'
/var/www/discourse/vendor/bundle/ruby/2.4.0/gems/activerecord-5.1.4/lib/active_record/connection_adapters/postgresql_adapter.rb:599:in `execute_and_clear'
/var/www/discourse/vendor/bundle/ruby/2.4.0/gems/activerecord-5.1.4/lib/active_record/connection_adapters/postgresql/database_statements.rb:92:in `exec_delete'
/var/www/discourse/vendor/bundle/ruby/2.4.0/gems/activerecord-5.1.4/lib/active_record/connection_adapters/abstract/database_statements.rb:140:in `update'
/var/www/discourse/vendor/bundle/ruby/2.4.0/gems/activerecord-5.1.4/lib/active_record/connection_adapters/abstract/query_cache.rb:17:in `update'
/var/www/discourse/vendor/bundle/ruby/2.4.0/gems/activerecord-5.1.4/lib/active_record/relation.rb:380:in `update_all'
/var/www/discourse/vendor/bundle/ruby/2.4.0/gems/activerecord-5.1.4/lib/active_record/persistence.rb:333:in `update_columns'
/var/www/discourse/vendor/bundle/ruby/2.4.0/gems/activerecord-5.1.4/lib/active_record/persistence.rb:306:in `update_column'
/var/www/discourse/app/models/user.rb:529:in `update_ip_address!'
/var/www/discourse/lib/auth/default_current_user_provider.rb:74:in `block in current_user'
/var/www/discourse/lib/scheduler/defer.rb:74:in `block in do_work'
/var/www/discourse/vendor/bundle/ruby/2.4.0/gems/rails_multisite-1.1.2/lib/rails_multisite/connection_management.rb:77:in `with_connection'
/var/www/discourse/lib/scheduler/defer.rb:72:in `do_work'
/var/www/discourse/lib/scheduler/defer.rb:61:in `block (2 levels) in start_thread'

looks to me like you are forwarding an incorrect field to ip address. How do you have IP Address forwarding configured?

Thank you Sam.
I didn’t configure that on purpose. Where can I check the config?

In the Azure setup… somewhere. That data is not coming from within Discourse.

It’s ip address from our company proxy. I don’t know how is the portal address added. Can we just remove the port info to accept this ip?

No, it isn’t an IP address, and that’s the problem. If you’ve got a proxy that’s including a port number in what’s supposed to be an IP address, then the proxy is broken, and you’ll want to get that fixed.

I haven’t find the root cause yet. But I find a defected solution. By adding the following configuration in app.yml. I make the input of ip_address valid.

       filename: /etc/nginx/conf.d/discourse.conf
       from: $proxy_add_x_forwarded_for
       to: $http_your_original_ip_header
       global: true

The defected part is that the user ip_addresses are all 127.0.0.1 now. It’s not perfect, but the 500 errors won’t throw finally.

According to RFC 7239 - Forwarded HTTP Extension

5.2. Forwarded For

The “for” parameter is used to disclose information about the client
that initiated the request and subsequent proxies in a chain of
proxies. When proxies choose to use the “for” parameter, its default
configuration SHOULD contain an obfuscated identifier as described in
Section 6.3. If the server receiving proxied requests requires some
address-based functionality, this parameter MAY instead contain an IP
address (and, potentially, a port number
). A third option is the
“unknown” identifier described in Section 6.2.

If your nginx setting is using $proxy_add_x_forwarded_for as the indicate of user ip. I think ip with port will be a valid input here. You may need to strip the port info out.

Well, this needs to be amended upstream then, see:

Feel free to raise a feature request for it at:

That RFC describes the format of the Forwarded: HTTP request header. The X-Forwarded-For header, which we’re examining, has different semantics.

Thanks for the correction.
And for Azure Gateway, it does have a different definition for x-forwarded-for.
See Frequently asked questions about Application Gateway | Microsoft Learn

Q. Does Application Gateway support x-forwarded-for headers?

Yes, Application Gateway inserts x-forwarded-for, x-forwarded-proto, and x-forwarded-port headers into the request forwarded to the backend. The format for x-forwarded-for header is a comma-separated list of IP:Port. The valid values for x-forwarded-proto are http or https. X-forwarded-port specifies the port at which the request reached at the Application Gateway.

I will seek solution in rack or Azure Gateway.
Thanks @sam also.

Hi,

Just to add to the conversation - this is standard practice in Azure Application Gateways, and it’s infuriating. They add the port to the forwarded-for header even though they also send it in the forwarded port header.

Two feedback/change requests on Microsoft which you can vote for;

Our “fix” is also the same as the above step, we’re manually setting the forwarded-for header to a generic ip as this caused lots of issues with users being logged out or the site not working properly.

Just out of interest, we are using a httpd redirect within the network, does anyone know if it is possible to rewrite the header and remove the port? Failing that can it be done in nginx (I am unfamiliar with nginx)?

I would look at submitting a PR if we get this fixed but the comments above seem to suggest the devs would prefer this fixing upstream - is that the case?

Great news! Microsoft have fixed their insufferable nonsense. Sort of!

https://azure.microsoft.com/en-gb/blog/rewrite-http-headers-with-azure-application-gateway/

明確にしておきます。これは SKU V2 の場合のみ機能します。したがって、V1 の AppGateway をお持ちの場合は、ヘッダーの書き換えはできません。

はい、私たちにとって、v2 が本番ワークロードで利用可能になった時点で、すぐにアップグレードする価値がありました。Azure ゲートウェイは奇妙な挙動を多く見せ、転送先 IP アドレスにポートを付加するなどの「独自の判断」が私たちを悩ませました。静的 VIP は素晴らしいです。

ゲートウェイ容量の自動スケーリングも優れており、すでに数回、私たちの窮地を救ってくれました。また、ゾーン冗長化が「ついに」実装されたことは非常に歓迎すべきことです。

移行には約 20 分かかりましたが、数百ものリスナーとある程度複雑なバックエンドプールを持っていながら、PowerShell スクリプトが支援してくれるため、非常に簡単でした。

ゲートウェイで証明書を使用しているかどうかわかりませんが、使用している場合、v2 は非常に非常に高速です。証明書の適用に 40 分も待つ必要はもうなく、今は数秒で完了します。

料金体系ははるかに複雑です(マイクロソフトは片手で与え、もう片方の手ですぐに取り戻す傾向があります)が、私たちにとっては、これまでのところ請求額は同じか、それ以下になっています。

ラッキーですね。コストが高いため、V2 プレビューが終了するとすぐに SKU V1 に切り替える必要がありました。

とにかく、この問題/ケースは既に マージされた PR によって上位で対応されており、rack 2.0 以降に含まれるはずです。しかし、この issue によると、現在のリリースではまだ含まれていないようです。

一時的には、CI/CD 中にパッチを適用してこの問題に対処しています。完璧ではありませんが、rack-/discourse の次のリリースで修正が確認されるまで、これで間に合います。

もし興味があれば、lib/auth/default_current_user_provider.rb 内の追加ポートを除去するために変更が必要な部分は以下の通りです:

if current_user && should_update_last_seen?
  u = current_user
  ip_port_split = request.ip.split(':')
  ip_only = ip_port_split.first
  Scheduler::Defer.later "Updating Last Seen" do
    u.update_last_seen!
    u.update_ip_address!(ip_only)
  end
end

この簡易修正を、そのファイル内の他の場所や他のファイル(email_controller.rb、006-mini_profiler.rb、request_tracker.rb)にある request.ip のすべての出現箇所に置き換えるのが良いアイデアかどうかはわかりませんが、私たちにとっては機能しています。
前述の通り、ビルド/CI プロセス中にパッチを適用することで、コードベースをクリーンに保ち、更新可能にすることができます。

より「きれいな」解決策があれば、ぜひご教示ください。

短いフォローアップです。
上記で提供した「修正」は、ここで説明されている のような問題を引き起こします。

当面は、この「分割」部分を次のように begin…rescue…end でラップすることで回避しています:

begin
  ip_port_split = request.ip.split(':')
  ip_only = ip_port_split.first
rescue
  ip_only = request.ip
end

よろしくお願いいたします
サッシャ

そのため、このコミット(rack 2.0.8 への更新)により、Azure Application Gateway v1 の ip:port 形式はもはや問題とならないはずです。