[SPARK-26118][Web UI] Introducing spark.ui.requestHeaderSize for setting HTTP requestHeaderSize #23090

attilapiros · 2018-11-19T20:44:58Z

What changes were proposed in this pull request?

Introducing spark.ui.requestHeaderSize for configuring Jetty's HTTP requestHeaderSize.
This way long authorization field does not lead to HTTP 431.

How was this patch tested?

Manually with curl (which version must be at least 7.55).

With the original default value (8k limit):

# Starting history server with default requestHeaderSize
$ ./sbin/start-history-server.sh
starting org.apache.spark.deploy.history.HistoryServer, logging to /Users/attilapiros/github/spark/logs/spark-attilapiros-org.apache.spark.deploy.history.HistoryServer-1-apiros-MBP.lan.out

# Creating huge header
$ echo -n "X-Custom-Header: " > cookie
$ printf 'A%.0s' {1..9500} >> cookie

# HTTP GET with huge header fails with 431 
$ curl  -H @cookie http://[email protected]:18080/
<h1>Bad Message 431</h1><pre>reason: Request Header Fields Too Large</pre> 

# The log contains the error
$ tail -1 /Users/attilapiros/github/spark/logs/spark-attilapiros-org.apache.spark.deploy.history.HistoryServer-1-apiros-MBP.lan.out
18/11/19 21:24:28 WARN HttpParser: Header is too large 8193>8192

After:

# Creating the history properties file with the increased requestHeaderSize
$ echo spark.ui.requestHeaderSize=10000 > history.properties

# Starting Spark History Server with the settings
$ ./sbin/start-history-server.sh --properties-file history.properties
starting org.apache.spark.deploy.history.HistoryServer, logging to /Users/attilapiros/github/spark/logs/spark-attilapiros-org.apache.spark.deploy.history.HistoryServer-1-apiros-MBP.lan.out

# HTTP GET with huge header gives back HTML5 (I have added here only just a part of the response)
$ curl  -H @cookie http://[email protected]:18080/
<!DOCTYPE html><html>
      <head>...
         <link rel="shortcut icon" href="/static/spark-logo-77x50px-hd.png"></link>
        <title>History Server</title>
      </head>
      <body>
...

squito

lgtm, just a small comment on the doc

docs/configuration.md

SparkQA · 2018-11-20T00:03:45Z

Test build #99018 has finished for PR 23090 at commit c6278be.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2018-11-20T00:15:51Z

Test build #99019 has finished for PR 23090 at commit 81334f4.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

HeartSaVioR

+1 Surprisingly I crafted quite similar patch in couple of days ago (not submitted yet on Apache side). LGTM.

SparkQA · 2018-11-20T02:06:44Z

Test build #99023 has finished for PR 23090 at commit a343f93.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

gengliangwang

LGTM

SparkQA · 2018-11-20T07:43:47Z

Test build #4434 has finished for PR 23090 at commit a343f93.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

gaborgsomogyi

LGTM

…ing HTTP requestHeaderSize ## What changes were proposed in this pull request? Introducing spark.ui.requestHeaderSize for configuring Jetty's HTTP requestHeaderSize. This way long authorization field does not lead to HTTP 413. ## How was this patch tested? Manually with curl (which version must be at least 7.55). With the original default value (8k limit): ```bash # Starting history server with default requestHeaderSize $ ./sbin/start-history-server.sh starting org.apache.spark.deploy.history.HistoryServer, logging to /Users/attilapiros/github/spark/logs/spark-attilapiros-org.apache.spark.deploy.history.HistoryServer-1-apiros-MBP.lan.out # Creating huge header $ echo -n "X-Custom-Header: " > cookie $ printf 'A%.0s' {1..9500} >> cookie # HTTP GET with huge header fails with 431 $ curl -H cookie http://458apiros-MBP.lan:18080/ <h1>Bad Message 431</h1><pre>reason: Request Header Fields Too Large</pre> # The log contains the error $ tail -1 /Users/attilapiros/github/spark/logs/spark-attilapiros-org.apache.spark.deploy.history.HistoryServer-1-apiros-MBP.lan.out 18/11/19 21:24:28 WARN HttpParser: Header is too large 8193>8192 ``` After: ```bash # Creating the history properties file with the increased requestHeaderSize $ echo spark.ui.requestHeaderSize=10000 > history.properties # Starting Spark History Server with the settings $ ./sbin/start-history-server.sh --properties-file history.properties starting org.apache.spark.deploy.history.HistoryServer, logging to /Users/attilapiros/github/spark/logs/spark-attilapiros-org.apache.spark.deploy.history.HistoryServer-1-apiros-MBP.lan.out # HTTP GET with huge header gives back HTML5 (I have added here only just a part of the response) $ curl -H cookie http://458apiros-MBP.lan:18080/ <!DOCTYPE html><html> <head>... <link rel="shortcut icon" href="/static/spark-logo-77x50px-hd.png"></link> <title>History Server</title> </head> <body> ... ``` Closes #23090 from attilapiros/JettyHeaderSize. Authored-by: “attilapiros” <[email protected]> Signed-off-by: Imran Rashid <[email protected]> (cherry picked from commit ab61ddb) Signed-off-by: Imran Rashid <[email protected]>

squito · 2018-11-20T14:57:25Z

merged to master / 2.4

dongjoon-hyun · 2018-11-20T17:43:40Z

Hi, @squito .
SPARK-26118 is marked as 'Improvement', we don't backport 'Improvement'.

attilapiros · 2018-11-20T17:48:51Z

@dongjoon-hyun, @squito it is my bad, this problem is present in earlier releases as well. If I can change the issue type I modify it to Bug.

dongjoon-hyun · 2018-11-20T17:54:59Z

I understand the situation. However, according to the content of the patch, it's an improvement to add a missing configuration, @attilapiros .

httpConfig.setRequestHeaderSize(conf.get(UI_REQUEST_HEADER_SIZE).toInt)

@rxin , @gatorsmile , @srowen . How do you think about this? If PMC allows this, I'd like to have this in the older branches, too. Otherwise, we need to revert this from branch-2.4.

srowen · 2018-11-20T19:04:54Z

Why not just hard-code a much higher limit? what's the consequence? in a generic public web server I can see that this defends against malicious or malformed requests, but that's much less an issue for an internal Spark server.

That is, would a user really need to vary this up and down and know enough to find this config?

squito · 2018-11-20T19:26:22Z

I think the default is pretty reasonable in most cases, in that this is the first time we've heard of someone hitting this limit. I'm not sure how high we would make it to get around this problem in general. And from the jetty docs: https://www.eclipse.org/jetty/javadoc/current/org/eclipse/jetty/server/HttpConfiguration.html#setRequestHeaderSize-int-

Larger headers will allow for more and/or larger cookies plus larger form content encoded in a URL. However, larger headers consume more memory and can make a server more vulnerable to denial of service attacks.

attilapiros · 2018-11-20T19:27:23Z

I would prefer to override this 8k limit when it is really necessary (and only with the extent which justified by the production system).

squito · 2018-11-20T19:41:53Z

btw I agree this was bad judgement on my part to only backport to 2.4, sorry abotu that and thanks for catching @dongjoon-hyun . I do think this fixes a bug (the ability to use the UI when you are a member of many user groups) and so should be backported further, though I would really be fine either way.

dongjoon-hyun · 2018-11-21T18:43:47Z

Thank you, @squito , @attilapiros , @srowen .

Then, @attilapiros , could you send backporting PRs against branch-2.3 and branch-2.2 please?

attilapiros · 2018-11-21T22:40:30Z

Thanks @dongjoon-hyun, of course I will create those PRs.

…ing HTTP requestHeaderSize Introducing spark.ui.requestHeaderSize for configuring Jetty's HTTP requestHeaderSize. This way long authorization field does not lead to HTTP 413. Manually with curl (which version must be at least 7.55). With the original default value (8k limit): ```bash $ ./sbin/start-history-server.sh starting org.apache.spark.deploy.history.HistoryServer, logging to /Users/attilapiros/github/spark/logs/spark-attilapiros-org.apache.spark.deploy.history.HistoryServer-1-apiros-MBP.lan.out $ echo -n "X-Custom-Header: " > cookie $ printf 'A%.0s' {1..9500} >> cookie $ curl -H cookie http://458apiros-MBP.lan:18080/ <h1>Bad Message 431</h1><pre>reason: Request Header Fields Too Large</pre> $ tail -1 /Users/attilapiros/github/spark/logs/spark-attilapiros-org.apache.spark.deploy.history.HistoryServer-1-apiros-MBP.lan.out 18/11/19 21:24:28 WARN HttpParser: Header is too large 8193>8192 ``` After: ```bash $ echo spark.ui.requestHeaderSize=10000 > history.properties $ ./sbin/start-history-server.sh --properties-file history.properties starting org.apache.spark.deploy.history.HistoryServer, logging to /Users/attilapiros/github/spark/logs/spark-attilapiros-org.apache.spark.deploy.history.HistoryServer-1-apiros-MBP.lan.out $ curl -H cookie http://458apiros-MBP.lan:18080/ <!DOCTYPE html><html> <head>... <link rel="shortcut icon" href="/static/spark-logo-77x50px-hd.png"></link> <title>History Server</title> </head> <body> ... ``` Closes apache#23090 from attilapiros/JettyHeaderSize. Authored-by: “attilapiros” <[email protected]> Signed-off-by: Imran Rashid <[email protected]> (cherry picked from commit ab61ddb)

attilapiros · 2018-11-22T01:22:13Z

Backport PR for 2.3: #23114

attilapiros · 2018-11-22T02:29:05Z

Backport PR for 2.2: #23115

Both 2.2 and 2.3 are retested.

…ing HTTP requestHeaderSize ## What changes were proposed in this pull request? Introducing spark.ui.requestHeaderSize for configuring Jetty's HTTP requestHeaderSize. This way long authorization field does not lead to HTTP 413. ## How was this patch tested? Manually with curl (which version must be at least 7.55). With the original default value (8k limit): ```bash # Starting history server with default requestHeaderSize $ ./sbin/start-history-server.sh starting org.apache.spark.deploy.history.HistoryServer, logging to /Users/attilapiros/github/spark/logs/spark-attilapiros-org.apache.spark.deploy.history.HistoryServer-1-apiros-MBP.lan.out # Creating huge header $ echo -n "X-Custom-Header: " > cookie $ printf 'A%.0s' {1..9500} >> cookie # HTTP GET with huge header fails with 431 $ curl -H cookie http://458apiros-MBP.lan:18080/ <h1>Bad Message 431</h1><pre>reason: Request Header Fields Too Large</pre> # The log contains the error $ tail -1 /Users/attilapiros/github/spark/logs/spark-attilapiros-org.apache.spark.deploy.history.HistoryServer-1-apiros-MBP.lan.out 18/11/19 21:24:28 WARN HttpParser: Header is too large 8193>8192 ``` After: ```bash # Creating the history properties file with the increased requestHeaderSize $ echo spark.ui.requestHeaderSize=10000 > history.properties # Starting Spark History Server with the settings $ ./sbin/start-history-server.sh --properties-file history.properties starting org.apache.spark.deploy.history.HistoryServer, logging to /Users/attilapiros/github/spark/logs/spark-attilapiros-org.apache.spark.deploy.history.HistoryServer-1-apiros-MBP.lan.out # HTTP GET with huge header gives back HTML5 (I have added here only just a part of the response) $ curl -H cookie http://458apiros-MBP.lan:18080/ <!DOCTYPE html><html> <head>... <link rel="shortcut icon" href="/static/spark-logo-77x50px-hd.png"></link> <title>History Server</title> </head> <body> ... ``` Closes apache#23090 from attilapiros/JettyHeaderSize. Authored-by: “attilapiros” <[email protected]> Signed-off-by: Imran Rashid <[email protected]>

## What changes were proposed in this pull request? `requestHeaderSize` is added in #23090 and applies to Spark + History server UI as well. Without debug log it's hard to find out on which side what configuration is used. In this PR I've added a log message which prints out the value. ## How was this patch tested? Manually checked log files. Closes #25045 from gaborgsomogyi/SPARK-26118. Authored-by: Gabor Somogyi <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]> (cherry picked from commit 0b6c2c2) Signed-off-by: Dongjoon Hyun <[email protected]>

…ing HTTP requestHeaderSize ## What changes were proposed in this pull request? Introducing spark.ui.requestHeaderSize for configuring Jetty's HTTP requestHeaderSize. This way long authorization field does not lead to HTTP 413. ## How was this patch tested? Manually with curl (which version must be at least 7.55). With the original default value (8k limit): ```bash # Starting history server with default requestHeaderSize $ ./sbin/start-history-server.sh starting org.apache.spark.deploy.history.HistoryServer, logging to /Users/attilapiros/github/spark/logs/spark-attilapiros-org.apache.spark.deploy.history.HistoryServer-1-apiros-MBP.lan.out # Creating huge header $ echo -n "X-Custom-Header: " > cookie $ printf 'A%.0s' {1..9500} >> cookie # HTTP GET with huge header fails with 431 $ curl -H cookie http://458apiros-MBP.lan:18080/ <h1>Bad Message 431</h1><pre>reason: Request Header Fields Too Large</pre> # The log contains the error $ tail -1 /Users/attilapiros/github/spark/logs/spark-attilapiros-org.apache.spark.deploy.history.HistoryServer-1-apiros-MBP.lan.out 18/11/19 21:24:28 WARN HttpParser: Header is too large 8193>8192 ``` After: ```bash # Creating the history properties file with the increased requestHeaderSize $ echo spark.ui.requestHeaderSize=10000 > history.properties # Starting Spark History Server with the settings $ ./sbin/start-history-server.sh --properties-file history.properties starting org.apache.spark.deploy.history.HistoryServer, logging to /Users/attilapiros/github/spark/logs/spark-attilapiros-org.apache.spark.deploy.history.HistoryServer-1-apiros-MBP.lan.out # HTTP GET with huge header gives back HTML5 (I have added here only just a part of the response) $ curl -H cookie http://458apiros-MBP.lan:18080/ <!DOCTYPE html><html> <head>... <link rel="shortcut icon" href="/static/spark-logo-77x50px-hd.png"></link> <title>History Server</title> </head> <body> ... ``` Closes apache#23090 from attilapiros/JettyHeaderSize. Authored-by: “attilapiros” <[email protected]> Signed-off-by: Imran Rashid <[email protected]> (cherry picked from commit ab61ddb) Signed-off-by: Imran Rashid <[email protected]>

## What changes were proposed in this pull request? `requestHeaderSize` is added in apache#23090 and applies to Spark + History server UI as well. Without debug log it's hard to find out on which side what configuration is used. In this PR I've added a log message which prints out the value. ## How was this patch tested? Manually checked log files. Closes apache#25045 from gaborgsomogyi/SPARK-26118. Authored-by: Gabor Somogyi <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]> (cherry picked from commit 0b6c2c2) Signed-off-by: Dongjoon Hyun <[email protected]>

attilapiros added 2 commits November 19, 2018 20:55

Introducing spark.ui.requestHeaderSize

c6278be

Remove unnecesary enters.

81334f4

attilapiros changed the title ~~[SPARK-26118][UI] Introducing spark.ui.requestHeaderSize for setting HTTP requestHeaderSize~~ [SPARK-26118][Web UII] Introducing spark.ui.requestHeaderSize for setting HTTP requestHeaderSize Nov 19, 2018

attilapiros changed the title ~~[SPARK-26118][Web UII] Introducing spark.ui.requestHeaderSize for setting HTTP requestHeaderSize~~ [SPARK-26118][Web UI] Introducing spark.ui.requestHeaderSize for setting HTTP requestHeaderSize Nov 19, 2018

squito approved these changes Nov 19, 2018

View reviewed changes

docs/configuration.md Outdated Show resolved Hide resolved

fix doc

a343f93

HeartSaVioR approved these changes Nov 20, 2018

View reviewed changes

gengliangwang approved these changes Nov 20, 2018

View reviewed changes

gaborgsomogyi approved these changes Nov 20, 2018

View reviewed changes

asfgit closed this in ab61ddb Nov 20, 2018

gaborgsomogyi mentioned this pull request Jul 3, 2019

[MINOR] Add requestHeaderSize debug log #25045

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-26118][Web UI] Introducing spark.ui.requestHeaderSize for setting HTTP requestHeaderSize #23090

[SPARK-26118][Web UI] Introducing spark.ui.requestHeaderSize for setting HTTP requestHeaderSize #23090

attilapiros commented Nov 19, 2018 •

edited

Loading

squito left a comment

SparkQA commented Nov 20, 2018

SparkQA commented Nov 20, 2018

HeartSaVioR left a comment •

edited

Loading

SparkQA commented Nov 20, 2018

gengliangwang left a comment

SparkQA commented Nov 20, 2018

gaborgsomogyi left a comment

squito commented Nov 20, 2018

dongjoon-hyun commented Nov 20, 2018

attilapiros commented Nov 20, 2018

dongjoon-hyun commented Nov 20, 2018 •

edited

Loading

srowen commented Nov 20, 2018

squito commented Nov 20, 2018

attilapiros commented Nov 20, 2018

squito commented Nov 20, 2018

dongjoon-hyun commented Nov 21, 2018

attilapiros commented Nov 21, 2018

attilapiros commented Nov 22, 2018

attilapiros commented Nov 22, 2018

[SPARK-26118][Web UI] Introducing spark.ui.requestHeaderSize for setting HTTP requestHeaderSize #23090

[SPARK-26118][Web UI] Introducing spark.ui.requestHeaderSize for setting HTTP requestHeaderSize #23090

Conversation

attilapiros commented Nov 19, 2018 • edited Loading

What changes were proposed in this pull request?

How was this patch tested?

squito left a comment

Choose a reason for hiding this comment

SparkQA commented Nov 20, 2018

SparkQA commented Nov 20, 2018

HeartSaVioR left a comment • edited Loading

Choose a reason for hiding this comment

SparkQA commented Nov 20, 2018

gengliangwang left a comment

Choose a reason for hiding this comment

SparkQA commented Nov 20, 2018

gaborgsomogyi left a comment

Choose a reason for hiding this comment

squito commented Nov 20, 2018

dongjoon-hyun commented Nov 20, 2018

attilapiros commented Nov 20, 2018

dongjoon-hyun commented Nov 20, 2018 • edited Loading

srowen commented Nov 20, 2018

squito commented Nov 20, 2018

attilapiros commented Nov 20, 2018

squito commented Nov 20, 2018

dongjoon-hyun commented Nov 21, 2018

attilapiros commented Nov 21, 2018

attilapiros commented Nov 22, 2018

attilapiros commented Nov 22, 2018

attilapiros commented Nov 19, 2018 •

edited

Loading

HeartSaVioR left a comment •

edited

Loading

dongjoon-hyun commented Nov 20, 2018 •

edited

Loading