Skip to content

Commit b237808

Browse files
committed
Fix fieldnotes: imports, parsing, security, style issues
- Add missing BadRequest import (fatal error on oc.pl) - Remove unused LogsCommon import - Fix isBase64() false positives: use round-trip check instead of regex - Add bounds check before accessing input bytes for encoding detection - Fix parseCSV record-splitting: use regex pattern (OC\w+,\d{4}-) to detect record starts instead of plain 'OC' prefix check - Truncate field_note.text to 255 chars (VARCHAR(255) column limit) - Remove nl2br() — draft logs should store plain text - Rename variables/methods to snake_case per project conventions - Restore save_user_coords in OkapiServiceRunner (kept per PR #628) - Rename getCacheTypes/getLogTypes to snake_case in installation service
1 parent 68dcca8 commit b237808

2 files changed

Lines changed: 36 additions & 27 deletions

File tree

okapi/services/apisrv/installation/WebService.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,15 @@ public static function call(OkapiRequest $request)
3636
if (Settings::get('OC_BRANCH') == 'oc.de') {
3737
$result['has_draft_logs'] = true;
3838
$result['has_lists'] = true;
39-
$result['cache_types'] = self::getCacheTypes();
40-
$result['log_types'] = self::getLogTypes();
39+
$result['cache_types'] = self::get_cache_types();
40+
$result['log_types'] = self::get_log_types();
4141

4242
}
4343

4444
return Okapi::formatted_response($request, $result);
4545
}
4646

47-
private static function getCacheTypes() {
47+
private static function get_cache_types() {
4848
$rs = Db::query("
4949
SELECT name
5050
FROM cache_type;
@@ -56,7 +56,7 @@ private static function getCacheTypes() {
5656
return $cache_types;
5757
}
5858

59-
private static function getLogTypes() {
59+
private static function get_log_types() {
6060
$rs = Db::query("
6161
SELECT name
6262
FROM log_types;

okapi/services/draftlogs/upload_fieldnotes/WebService.php

Lines changed: 32 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@
22

33
namespace okapi\services\draftlogs\upload_fieldnotes;
44

5+
use okapi\core\Exception\BadRequest;
56
use okapi\core\Exception\InvalidParam;
67
use okapi\core\Exception\ParamMissing;
78
use okapi\core\Db;
89
use okapi\core\Okapi;
910
use okapi\core\OkapiServiceRunner;
1011
use okapi\core\Request\OkapiInternalRequest;
1112
use okapi\core\Request\OkapiRequest;
12-
use okapi\services\logs\LogsCommon;
1313
use okapi\Settings;
1414

1515
class WebService
@@ -44,7 +44,7 @@ public static function call(OkapiRequest $request)
4444

4545
//First figure out whether it is base64 or not. If it is, decode it.
4646

47-
if (self::isBase64($field_notes)) {
47+
if (self::is_base64($field_notes)) {
4848
$input = base64_decode($field_notes, true);
4949
} else {
5050
$input = $field_notes;
@@ -56,6 +56,10 @@ public static function call(OkapiRequest $request)
5656
// it is the safest approach to do this manually with just a few lines of code which can be understood
5757
// by looking at it at a glance.
5858

59+
if (strlen($input) < 3) {
60+
throw new InvalidParam('field_notes', "Input data is too short to be valid.");
61+
}
62+
5963
switch (true) {
6064
case $input[0] === "\xEF" && $input[1] === "\xBB" && $input[2] === "\xBF": // UTF-8 BOM
6165
$output = substr($input, 3);
@@ -96,11 +100,11 @@ public static function call(OkapiRequest $request)
96100
throw new InvalidParam('Type', 'Invalid log type provided.');
97101
}
98102

99-
$dateString = strtotime($n['date']);
100-
if ($dateString === false) {
103+
$date_timestamp = strtotime($n['date']);
104+
if ($date_timestamp === false) {
101105
throw new InvalidParam('`Date` field in log record', "Input data not recognized.");
102106
} else {
103-
$date = date("Y-m-d H:i:s", $dateString);
107+
$date = date("Y-m-d H:i:s", $date_timestamp);
104108
}
105109

106110
$user_id = $request->token->user_id;
@@ -130,9 +134,9 @@ public static function call(OkapiRequest $request)
130134
// totalRecords.
131135

132136
$result = array(
133-
'success' => true,
134-
'totalRecords' => $notes['totalRecords'],
135-
'processedRecords' => $notes['processedRecords']
137+
'success' => true,
138+
'total_records' => $notes['total_records'],
139+
'processed_records' => $notes['processed_records']
136140
);
137141
return Okapi::formatted_response($request, $result);
138142
}
@@ -155,14 +159,14 @@ public static function call(OkapiRequest $request)
155159

156160
private static function parse_notes($field_notes)
157161
{
158-
$lines = self::parseCSV($field_notes);
162+
$lines = self::parse_csv($field_notes);
159163
$submittable_logtype_names = Okapi::get_submittable_logtype_names();
160-
$records = [];
161-
$totalRecords = 0;
162-
$processedRecords = 0;
164+
$records = [];
165+
$total_records = 0;
166+
$processed_records = 0;
163167

164168
foreach ($lines as $line) {
165-
$totalRecords++;
169+
$total_records++;
166170
$line = trim($line);
167171
$fields = str_getcsv($line);
168172

@@ -172,17 +176,17 @@ private static function parse_notes($field_notes)
172176

173177
if (!in_array($type, $submittable_logtype_names)) continue;
174178

175-
$log = nl2br($fields[3]);
179+
$log = mb_substr($fields[3], 0, 255);
176180

177181
$records[] = [
178182
'code' => $code,
179183
'date' => $date,
180184
'type' => $type,
181185
'log' => $log,
182186
];
183-
$processedRecords++;
187+
$processed_records++;
184188
}
185-
return ['success' => true, 'records' => $records, 'totalRecords' => $totalRecords, 'processedRecords' => $processedRecords];
189+
return ['success' => true, 'records' => $records, 'total_records' => $total_records, 'processed_records' => $processed_records];
186190
}
187191

188192

@@ -196,25 +200,26 @@ private static function parse_notes($field_notes)
196200
//
197201
// In this function we ony take log records which start with "OC" (for opencaching.de)
198202

199-
private static function parseCSV($fieldnotes)
203+
private static function parse_csv($field_notes)
200204
{
201205
$output = [];
202206
$buffer = '';
203207
$start = true;
204208

205-
$lines = explode("\n", $fieldnotes);
209+
$lines = explode("\n", $field_notes);
206210
$lines = array_filter($lines); // Drop empty lines
207211

208212
foreach ($lines as $line) {
209213
if ($start) {
210214
$buffer = $line;
211215
$start = false;
212216
} else {
213-
if (strpos($line, 'OC') !== 0) {
214-
$buffer .= "\n" . $line;
215-
} else {
217+
// A new record starts with a cache code followed by an ISO date
218+
if (preg_match('/^OC\w+,\d{4}-/', $line)) {
216219
$output[] = trim($buffer);
217220
$buffer = $line;
221+
} else {
222+
$buffer .= "\n" . $line;
218223
}
219224
}
220225
}
@@ -228,9 +233,13 @@ private static function parseCSV($fieldnotes)
228233
// ------------------------------------------------------------------
229234
// Check whether a string ($s) is base64 encoded or not.
230235

231-
private static function isBase64($s)
236+
private static function is_base64($s)
232237
{
233-
return (bool) preg_match('/^[a-zA-Z0-9\/\r\n+]*={0,2}$/', $s);
238+
$decoded = base64_decode($s, true);
239+
if ($decoded === false) {
240+
return false;
241+
}
242+
return base64_encode($decoded) === $s;
234243
}
235244

236245
// ------------------------------------------------------------------

0 commit comments

Comments
 (0)