Skip to content

Commit 2bbce8e

Browse files
dvikandvikan
authored
refactor: general code base refactor (RSS-Bridge#2950)
* refactor * fix: bug in previous refactor * chore: exclude phpcompat sniff due to bug in phpcompat * fix: do not leak absolute paths * refactor/fix: batch extensions checking, fix DOS issue
1 parent b042412 commit 2bbce8e

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+677
-825
lines changed

‎actions/ConnectivityAction.php

Lines changed: 8 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -28,23 +28,21 @@ class ConnectivityAction implements ActionInterface
2828

2929
public function __construct()
3030
{
31-
$this->bridgeFactory = new \BridgeFactory();
31+
$this->bridgeFactory = new BridgeFactory();
3232
}
3333

3434
public function execute(array $request)
3535
{
3636
if (!Debug::isEnabled()) {
37-
returnError('This action is only available in debug mode!', 400);
37+
throw new \Exception('This action is only available in debug mode!');
3838
}
3939

4040
if (!isset($request['bridge'])) {
41-
$this->returnEntryPage();
41+
print render_template('connectivity.html.php');
4242
return;
4343
}
4444

45-
$bridgeName = $request['bridge'];
46-
47-
$bridgeClassName = $this->bridgeFactory->sanitizeBridgeName($bridgeName);
45+
$bridgeClassName = $this->bridgeFactory->sanitizeBridgeName($request['bridge']);
4846

4947
if ($bridgeClassName === null) {
5048
throw new \InvalidArgumentException('Bridge name invalid!');
@@ -53,45 +51,22 @@ public function execute(array $request)
5351
$this->reportBridgeConnectivity($bridgeClassName);
5452
}
5553

56-
/**
57-
* Generates a report about the bridge connectivity status and sends it back
58-
* to the user.
59-
*
60-
* The report is generated as Json-formatted string in the format
61-
* {
62-
* "bridge": "<bridge-name>",
63-
* "successful": true/false
64-
* }
65-
*
66-
* @param class-string<BridgeInterface> $bridgeClassName Name of the bridge to generate the report for
67-
* @return void
68-
*/
6954
private function reportBridgeConnectivity($bridgeClassName)
7055
{
7156
if (!$this->bridgeFactory->isWhitelisted($bridgeClassName)) {
72-
header('Content-Type: text/html');
73-
returnServerError('Bridge is not whitelisted!');
57+
throw new \Exception('Bridge is not whitelisted!');
7458
}
7559

76-
header('Content-Type: text/json');
77-
7860
$retVal = [
7961
'bridge' => $bridgeClassName,
8062
'successful' => false,
8163
'http_code' => 200,
8264
];
8365

8466
$bridge = $this->bridgeFactory->create($bridgeClassName);
85-
86-
if ($bridge === false) {
87-
echo json_encode($retVal);
88-
return;
89-
}
90-
9167
$curl_opts = [
9268
CURLOPT_CONNECTTIMEOUT => 5
9369
];
94-
9570
try {
9671
$reply = getContents($bridge::URI, [], $curl_opts, true);
9772

@@ -101,45 +76,11 @@ private function reportBridgeConnectivity($bridgeClassName)
10176
$retVal['http_code'] = 301;
10277
}
10378
}
104-
} catch (Exception $e) {
79+
} catch (\Exception $e) {
10580
$retVal['successful'] = false;
10681
}
10782

108-
echo json_encode($retVal);
109-
}
110-
111-
private function returnEntryPage()
112-
{
113-
echo <<<EOD
114-
<!DOCTYPE html>
115-
116-
<html>
117-
<head>
118-
<link rel="stylesheet" href="static/bootstrap.min.css">
119-
<link
120-
rel="stylesheet"
121-
href="https://use.fontawesome.com/releases/v5.6.3/css/all.css"
122-
integrity="sha384-UHRtZLI+pbxtHCWp1t77Bi1L4ZtiqrqD80Kn4Z8NTSRyMA2Fd33n5dQ8lWUE00s/"
123-
crossorigin="anonymous">
124-
<link rel="stylesheet" href="static/connectivity.css">
125-
<script src="static/connectivity.js" type="text/javascript"></script>
126-
</head>
127-
<body>
128-
<div id="main-content" class="container">
129-
<div class="progress">
130-
<div class="progress-bar" role="progressbar" aria-valuenow="75" aria-valuemin="0" aria-valuemax="100"></div>
131-
</div>
132-
<div id="status-message" class="sticky-top alert alert-primary alert-dismissible fade show" role="alert">
133-
<i id="status-icon" class="fas fa-sync"></i>
134-
<span>...</span>
135-
<button type="button" class="close" data-dismiss="alert" aria-label="Close" onclick="stopConnectivityChecks()">
136-
<span aria-hidden="true">&times;</span>
137-
</button>
138-
</div>
139-
<input type="text" class="form-control" id="search" onkeyup="search()" placeholder="Search for bridge..">
140-
</div>
141-
</body>
142-
</html>
143-
EOD;
83+
header('Content-Type: text/json');
84+
print Json::encode($retVal);
14485
}
14586
}

‎actions/DetectAction.php

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,17 @@ class DetectAction implements ActionInterface
1616
{
1717
public function execute(array $request)
1818
{
19-
$targetURL = $request['url']
20-
or returnClientError('You must specify a url!');
19+
$targetURL = $request['url'] ?? null;
20+
$format = $request['format'] ?? null;
2121

22-
$format = $request['format']
23-
or returnClientError('You must specify a format!');
22+
if (!$targetURL) {
23+
throw new \Exception('You must specify a url!');
24+
}
25+
if (!$format) {
26+
throw new \Exception('You must specify a format!');
27+
}
2428

25-
$bridgeFactory = new \BridgeFactory();
29+
$bridgeFactory = new BridgeFactory();
2630

2731
foreach ($bridgeFactory->getBridgeClassNames() as $bridgeClassName) {
2832
if (!$bridgeFactory->isWhitelisted($bridgeClassName)) {
@@ -31,10 +35,6 @@ public function execute(array $request)
3135

3236
$bridge = $bridgeFactory->create($bridgeClassName);
3337

34-
if ($bridge === false) {
35-
continue;
36-
}
37-
3838
$bridgeParams = $bridge->detectParameters($targetURL);
3939

4040
if (is_null($bridgeParams)) {
@@ -45,9 +45,9 @@ public function execute(array $request)
4545
$bridgeParams['format'] = $format;
4646

4747
header('Location: ?action=display&' . http_build_query($bridgeParams), true, 301);
48-
exit;
48+
return;
4949
}
5050

51-
returnClientError('No bridge found for given URL: ' . $targetURL);
51+
throw new \Exception('No bridge found for given URL: ' . $targetURL);
5252
}
5353
}

‎actions/DisplayAction.php

Lines changed: 23 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ class DisplayAction implements ActionInterface
1616
{
1717
public function execute(array $request)
1818
{
19-
$bridgeFactory = new \BridgeFactory();
19+
$bridgeFactory = new BridgeFactory();
2020

2121
$bridgeClassName = null;
2222
if (isset($request['bridge'])) {
@@ -27,16 +27,14 @@ public function execute(array $request)
2727
throw new \InvalidArgumentException('Bridge name invalid!');
2828
}
2929

30-
$format = $request['format']
31-
or returnClientError('You must specify a format!');
32-
33-
// whitelist control
30+
$format = $request['format'] ?? null;
31+
if (!$format) {
32+
throw new \Exception('You must specify a format!');
33+
}
3434
if (!$bridgeFactory->isWhitelisted($bridgeClassName)) {
35-
throw new \Exception('This bridge is not whitelisted', 401);
36-
die;
35+
throw new \Exception('This bridge is not whitelisted');
3736
}
3837

39-
// Data retrieval
4038
$bridge = $bridgeFactory->create($bridgeClassName);
4139
$bridge->loadConfiguration();
4240

@@ -47,14 +45,12 @@ public function execute(array $request)
4745
define('NOPROXY', true);
4846
}
4947

50-
// Cache timeout
51-
$cache_timeout = -1;
5248
if (array_key_exists('_cache_timeout', $request)) {
5349
if (!CUSTOM_CACHE_TIMEOUT) {
5450
unset($request['_cache_timeout']);
5551
$uri = parse_url($_SERVER['REQUEST_URI'], PHP_URL_PATH) . '?' . http_build_query($request);
5652
header('Location: ' . $uri, true, 301);
57-
exit;
53+
return;
5854
}
5955

6056
$cache_timeout = filter_var($request['_cache_timeout'], FILTER_VALIDATE_INT);
@@ -93,7 +89,6 @@ public function execute(array $request)
9389
)
9490
);
9591

96-
// Initialize cache
9792
$cacheFactory = new CacheFactory();
9893

9994
$cache = $cacheFactory->create();
@@ -109,43 +104,42 @@ public function execute(array $request)
109104
$mtime !== false
110105
&& (time() - $cache_timeout < $mtime)
111106
&& !Debug::isEnabled()
112-
) { // Load cached data
107+
) {
108+
// Load cached data
113109
// Send "Not Modified" response if client supports it
114110
// Implementation based on https://stackoverflow.com/a/10847262
115111
if (isset($_SERVER['HTTP_IF_MODIFIED_SINCE'])) {
116112
$stime = strtotime($_SERVER['HTTP_IF_MODIFIED_SINCE']);
117113

118-
if ($mtime <= $stime) { // Cached data is older or same
114+
if ($mtime <= $stime) {
115+
// Cached data is older or same
119116
header('Last-Modified: ' . gmdate('D, d M Y H:i:s ', $mtime) . 'GMT', true, 304);
120-
exit;
117+
return;
121118
}
122119
}
123120

124121
$cached = $cache->loadData();
125122

126123
if (isset($cached['items']) && isset($cached['extraInfos'])) {
127124
foreach ($cached['items'] as $item) {
128-
$items[] = new \FeedItem($item);
125+
$items[] = new FeedItem($item);
129126
}
130127

131128
$infos = $cached['extraInfos'];
132129
}
133-
} else { // Collect new data
130+
} else {
131+
// Collect new data
134132
try {
135133
$bridge->setDatas($bridge_params);
136134
$bridge->collectData();
137135

138136
$items = $bridge->getItems();
139137

140-
// Transform "legacy" items to FeedItems if necessary.
141-
// Remove this code when support for "legacy" items ends!
142138
if (isset($items[0]) && is_array($items[0])) {
143139
$feedItems = [];
144-
145140
foreach ($items as $item) {
146-
$feedItems[] = new \FeedItem($item);
141+
$feedItems[] = new FeedItem($item);
147142
}
148-
149143
$items = $feedItems;
150144
}
151145

@@ -158,18 +152,16 @@ public function execute(array $request)
158152
} catch (\Throwable $e) {
159153
error_log($e);
160154

161-
if (logBridgeError($bridge::NAME, $e->getCode()) >= Configuration::getConfig('error', 'report_limit')) {
155+
$errorCount = logBridgeError($bridge::NAME, $e->getCode());
156+
157+
if ($errorCount >= Configuration::getConfig('error', 'report_limit')) {
162158
if (Configuration::getConfig('error', 'output') === 'feed') {
163-
$item = new \FeedItem();
159+
$item = new FeedItem();
164160

165161
// Create "new" error message every 24 hours
166162
$request['_error_time'] = urlencode((int)(time() / 86400));
167163

168-
$message = sprintf(
169-
'Bridge returned error %s! (%s)',
170-
$e->getCode(),
171-
$request['_error_time']
172-
);
164+
$message = sprintf('Bridge returned error %s! (%s)', $e->getCode(), $request['_error_time']);
173165
$item->setTitle($message);
174166

175167
$item->setURI(
@@ -205,8 +197,8 @@ public function execute(array $request)
205197
}
206198

207199
$cache->saveData([
208-
'items' => array_map(function ($i) {
209-
return $i->toArray();
200+
'items' => array_map(function (FeedItem $item) {
201+
return $item->toArray();
210202
}, $items),
211203
'extraInfos' => $infos
212204
]);

‎actions/ListAction.php

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,27 +16,17 @@ class ListAction implements ActionInterface
1616
{
1717
public function execute(array $request)
1818
{
19-
$list = new StdClass();
19+
$list = new \stdClass();
2020
$list->bridges = [];
2121
$list->total = 0;
2222

23-
$bridgeFactory = new \BridgeFactory();
23+
$bridgeFactory = new BridgeFactory();
2424

2525
foreach ($bridgeFactory->getBridgeClassNames() as $bridgeClassName) {
2626
$bridge = $bridgeFactory->create($bridgeClassName);
2727

28-
if ($bridge === false) { // Broken bridge, show as inactive
29-
$list->bridges[$bridgeClassName] = [
30-
'status' => 'inactive'
31-
];
32-
33-
continue;
34-
}
35-
36-
$status = $bridgeFactory->isWhitelisted($bridgeClassName) ? 'active' : 'inactive';
37-
3828
$list->bridges[$bridgeClassName] = [
39-
'status' => $status,
29+
'status' => $bridgeFactory->isWhitelisted($bridgeClassName) ? 'active' : 'inactive',
4030
'uri' => $bridge->getURI(),
4131
'donationUri' => $bridge->getDonationURI(),
4232
'name' => $bridge->getName(),
@@ -50,6 +40,6 @@ public function execute(array $request)
5040
$list->total = count($list->bridges);
5141

5242
header('Content-Type: application/json');
53-
echo json_encode($list, JSON_PRETTY_PRINT);
43+
print Json::encode($list);
5444
}
5545
}

0 commit comments

Comments
 (0)