fossar / selfoss

multipurpose rss reader, live stream, mashup, aggregation web application
https://selfoss.aditu.de
GNU General Public License v3.0
2.38k stars 345 forks source link

HTTP 500 when loading more items #1173

Closed dgsiegel closed 4 years ago

dgsiegel commented 4 years ago

When loading more items on the "newest" view - either via the more button or via autoloading - the request results in a 500 Internal Server Error.

The reason is the following: All items do have their datetime stored as a data attribute, e.g.

<div id="entry245606" data-entry-id="245606" data-entry-source="168" data-entry-datetime="2020-02-19T20:36:41+01:00" class="entry timestamped" role="article">

This then gets appended as a parameter:

?offset=0&fromDatetime=2020-02-19T01%3A00%3A00%2B01%3A00&fromId=245539&itemsPerPage=100&search=&type=newest&tag=fun&source=&sourcesNav=false&ajax=true

Somewhere the + symbol gets dropped, and inside the PHP code the date gets stripped to e.g. 2020-02-19T20:36:41 01:00, which of course is not a valid date and results in the following error:

[2020-03-01 22:03:52] selfoss.ERROR: DateTime::__construct(): Failed to parse time string (2020-02-19T01:00:00 01:00) at position 20 (0): Double time specification {"exception":"[object] (Exception(code: 0): DateTime::__construct(): Failed to parse time string (2020-02-19T01:00:00 01:00) at position 20 (0): Double time specification at daos/sqlite/Statements.php:58)
[stacktrace]
#0 /daos/sqlite/Statements.php(58): DateTime->__construct('2020-02-19T01:0...')
#1 /daos/mysql/Items.php(264): daos\\sqlite\\Statements::datetime('2020-02-19T01:0...')
#2 /daos/Items.php(78): daos\\mysql\\Items->get(Array)
#3 /controllers/Index.php(218): daos\\Items->get(Array)
#4 /controllers/Index.php(52): controllers\\Index->loadItems(Array, Array)
#5 /vendor/bcosca/fatfree-core/base.php(1806): controllers\\Index->home(Object(Base), Array, 'controllers\\\\Ind...')
#6 /vendor/bcosca/fatfree-core/base.php(1627): Base->call(Array, Array, Array)
#7 /index.php(80): Base->run()
#8 {main}
"}

This bug happens in both 2.18 as well as in the master branch.

dgsiegel commented 4 years ago

Using

data-entry-datetime="<?= $this->item['datetime']; ?>"

instead of

data-entry-datetime="<?= $this->viewHelper->date_iso8601($this->item['datetime']); ?>"

in src/templates/item.phtml:24 resolves this as well, as it removes the timezone part.

jtojnar commented 4 years ago

Web servers usually parse + as a space but here it is already URL-encoded so it should not be an issue. Will investigate further.

jtojnar commented 4 years ago

The .htaccess is unnecessarily complex so I simplified it slightly in 71b599fc0a929ae0a083ca7c988b932380a9af33 but I cannot reproduce the issue on Apache httpd.

What web server do you use?

dgsiegel commented 4 years ago

I use Lighttpd. I did some quick checks and as far as I could reproduce, the full and valid date gets retrieved in index.php on e.g. $_SERVER or $_GET.

jtojnar commented 4 years ago

Do you mean that

--- a/index.php
+++ b/index.php
@@ -1,5 +1,7 @@
 <?php

+var_dump($_GET['fromDatetime']); die;
+
 require __DIR__ . '/src/common.php';

 // Load custom language

shows the + character?

We just use $_GET directly

https://github.com/SSilence/selfoss/blob/71b599fc0a929ae0a083ca7c988b932380a9af33/src/controllers/Index.php#L24

so if you see + in the index.php but ` incontrollers/Index.php` then I would suspect the f3 framework we use. It does some pretty weird things sometimes.

dgsiegel commented 4 years ago

You're right, it does not:

string(25) "2020-02-19T01:00:00 01:00"

Only $_SERVER seems to be correct:

["REQUEST_URI"]=>
  string(151) "/?offset=0&fromDatetime=2020-02-19T01%3A00%3A00%2B01%3A00&fromId=245539&itemsPerPage=100&search=&type=newest&tag=fun&source=&sourcesNav=false&ajax=true"
["QUERY_STRING"]=>
  string(141) "offset=0&fromDatetime=2020-02-19T01:00:00+01:00&fromId=245539&itemsPerPage=100&search=&type=newest&tag=fun&source=&sourcesNav=false&ajax=true"

Your hint lead me down to review my Lighttpd configuration, and indeed the culprit was server.http-parseopts. more specifically the url-normalize-required option. Removing that leaves the + sign untouched.

Apparently this is a Lighttpd bug, which was already patched upstream:

https://redmine.lighttpd.net/issues/2999

Thanks for your help!