Skip to content

Commit e9b3ed0

Browse files
tstacktstack
committed
[session] fix hash for JSON logs
Should've been using the raw JSON line instead of the formatted one. Add a hack to session loading so old bookmarks survive.
1 parent cf2419b commit e9b3ed0

14 files changed

+86
-43
lines changed

‎src/log_format.cc‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1000,6 +1000,7 @@ struct json_log_userdata {
10001000
std::optional<std::string> jlu_subid;
10011001
exttm jlu_exttm;
10021002
size_t jlu_read_order_index{0};
1003+
subline_options jlu_subline_opts;
10031004
};
10041005

10051006
static int read_json_field(yajlpp_parse_context* ypc,
@@ -1970,7 +1971,7 @@ external_log_format::annotate(logfile* lf,
19701971

19711972
line.erase_ansi();
19721973
if (this->elf_type != elf_type_t::ELF_TYPE_TEXT) {
1973-
if (this->jlf_cached_full) {
1974+
if (this->jlf_cached_opts.full_message) {
19741975
values = this->jlf_line_values;
19751976
sa = this->jlf_line_attrs;
19761977
} else {
@@ -2368,13 +2369,15 @@ rewrite_json_field(yajlpp_parse_context* ypc,
23682369
jlu->jlu_format->lf_date_time.relock(ls);
23692370
}
23702371
}
2371-
if (jlu->jlu_exttm.et_flags & ETF_ZONE_SET
2372-
&& jlu->jlu_format->lf_date_time.dts_zoned_to_local)
2373-
{
2374-
jlu->jlu_exttm.et_flags &= ~ETF_Z_IS_UTC;
2372+
if (!jlu->jlu_subline_opts.hash_hack) {
2373+
if (jlu->jlu_exttm.et_flags & ETF_ZONE_SET
2374+
&& jlu->jlu_format->lf_date_time.dts_zoned_to_local)
2375+
{
2376+
jlu->jlu_exttm.et_flags &= ~ETF_Z_IS_UTC;
2377+
}
2378+
jlu->jlu_exttm.et_gmtoff
2379+
= jlu->jlu_format->lf_date_time.dts_local_offset_cache;
23752380
}
2376-
jlu->jlu_exttm.et_gmtoff
2377-
= jlu->jlu_format->lf_date_time.dts_local_offset_cache;
23782381
jlu->jlu_format->lf_date_time.ftime(
23792382
time_buf,
23802383
sizeof(time_buf),
@@ -2429,19 +2432,21 @@ rewrite_json_field(yajlpp_parse_context* ypc,
24292432
void
24302433
external_log_format::get_subline(const logline& ll,
24312434
shared_buffer_ref& sbr,
2432-
bool full_message)
2435+
subline_options opts)
24332436
{
24342437
if (this->elf_type == elf_type_t::ELF_TYPE_TEXT) {
24352438
return;
24362439
}
24372440

24382441
if (this->jlf_cached_offset != ll.get_offset()
2439-
|| this->jlf_cached_full != full_message)
2442+
|| this->jlf_cached_opts != opts)
24402443
{
24412444
auto& ypc = *(this->jlf_parse_context);
24422445
yajl_handle handle = this->jlf_yajl_handle.get();
24432446
json_log_userdata jlu(sbr, nullptr);
24442447

2448+
jlu.jlu_subline_opts = opts;
2449+
24452450
this->jlf_share_manager.invalidate_refs();
24462451
this->jlf_cached_line.clear();
24472452
this->jlf_line_values.clear();
@@ -2615,7 +2620,7 @@ external_log_format::get_subline(const logline& ll,
26152620
this->json_append(jfe, vd, str);
26162621
}
26172622

2618-
if (nl_pos == std::string::npos || full_message) {
2623+
if (nl_pos == std::string::npos || opts.full_message) {
26192624
lr.lr_end = this->jlf_cached_line.size();
26202625
} else {
26212626
lr.lr_end = lr.lr_start + nl_pos;
@@ -2838,7 +2843,7 @@ external_log_format::get_subline(const logline& ll,
28382843
}
28392844
this->jlf_line_offsets.push_back(this->jlf_cached_line.size());
28402845
this->jlf_cached_offset = ll.get_offset();
2841-
this->jlf_cached_full = full_message;
2846+
this->jlf_cached_opts = opts;
28422847
}
28432848

28442849
off_t this_off = 0, next_off = 0;
@@ -2861,7 +2866,7 @@ external_log_format::get_subline(const logline& ll,
28612866
}
28622867
}
28632868

2864-
if (full_message) {
2869+
if (opts.full_message) {
28652870
sbr.share(this->jlf_share_manager,
28662871
&this->jlf_cached_line[0],
28672872
this->jlf_cached_line.size());

‎src/log_format.hh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -524,7 +524,7 @@ public:
524524

525525
virtual void get_subline(const logline& ll,
526526
shared_buffer_ref& sbr,
527-
bool full_message = false)
527+
subline_options opts = subline_options{})
528528
{
529529
}
530530

‎src/log_format_ext.hh‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ public:
182182

183183
void get_subline(const logline& ll,
184184
shared_buffer_ref& sbr,
185-
bool full_message) override;
185+
subline_options opts) override;
186186

187187
std::shared_ptr<log_vtab_impl> get_vtab_impl() const override;
188188

@@ -454,7 +454,7 @@ public:
454454

455455
off_t jlf_cached_offset{-1};
456456
line_range jlf_cached_sub_range;
457-
bool jlf_cached_full{false};
457+
subline_options jlf_cached_opts{};
458458
std::vector<off_t> jlf_line_offsets;
459459
std::vector<char> jlf_cached_line;
460460
string_attrs_t jlf_line_attrs;

‎src/log_format_fwd.hh‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,4 +431,22 @@ struct format_partition_def {
431431
log_level_t fpd_level{LEVEL_UNKNOWN};
432432
};
433433

434+
struct subline_options {
435+
friend bool operator==(const subline_options& lhs,
436+
const subline_options& rhs)
437+
{
438+
return lhs.full_message == rhs.full_message
439+
&& lhs.hash_hack == rhs.hash_hack;
440+
}
441+
442+
friend bool operator!=(const subline_options& lhs,
443+
const subline_options& rhs)
444+
{
445+
return !(lhs == rhs);
446+
}
447+
448+
bool full_message{false};
449+
bool hash_hack{false};
450+
};
451+
434452
#endif

‎src/log_format_impls.cc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ class piper_log_format : public log_format {
9090

9191
void get_subline(const logline& ll,
9292
shared_buffer_ref& sbr,
93-
bool full_message) override
93+
subline_options opts) override
9494
{
9595
this->plf_cached_line.resize(TIMESTAMP_SIZE);
9696
auto tlen = sql_strftime(this->plf_cached_line.data(),
@@ -1045,7 +1045,7 @@ class bro_log_format : public log_format {
10451045

10461046
void get_subline(const logline& ll,
10471047
shared_buffer_ref& sbr,
1048-
bool full_message) override
1048+
subline_options opts) override
10491049
{
10501050
}
10511051

@@ -1879,7 +1879,7 @@ class w3c_log_format : public log_format {
18791879

18801880
void get_subline(const logline& ll,
18811881
shared_buffer_ref& sbr,
1882-
bool full_message) override
1882+
subline_options opts) override
18831883
{
18841884
}
18851885

‎src/logfile.cc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1395,20 +1395,20 @@ logfile::rebuild_index(std::optional<ui_clock::time_point> deadline)
13951395
}
13961396

13971397
Result<shared_buffer_ref, std::string>
1398-
logfile::read_line(logfile::iterator ll)
1398+
logfile::read_line(iterator ll, subline_options opts)
13991399
{
14001400
try {
14011401
auto get_range_res = this->get_file_range(ll, false);
14021402
return this->lf_line_buffer.read_range(get_range_res)
1403-
.map([&ll, &get_range_res, this](auto sbr) {
1403+
.map([&ll, &get_range_res, &opts, this](auto sbr) {
14041404
sbr.rtrim(is_line_ending);
14051405
if (!get_range_res.fr_metadata.m_valid_utf) {
14061406
scrub_to_utf8(sbr.get_writable_data(), sbr.length());
14071407
sbr.get_metadata().m_valid_utf = true;
14081408
}
14091409

14101410
if (this->lf_format != nullptr) {
1411-
this->lf_format->get_subline(*ll, sbr);
1411+
this->lf_format->get_subline(*ll, sbr, opts);
14121412
}
14131413

14141414
return sbr;
@@ -1531,7 +1531,7 @@ logfile::read_full_message(const_iterator ll,
15311531
msg_out.get_metadata() = range_for_line.fr_metadata;
15321532
}
15331533
if (this->lf_format.get() != nullptr) {
1534-
this->lf_format->get_subline(*ll, msg_out, true);
1534+
this->lf_format->get_subline(*ll, msg_out, {true});
15351535
}
15361536
} catch (const line_buffer::error& e) {
15371537
log_error("failed to read line");

‎src/logfile.hh‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,8 @@ public:
274274

275275
struct timeval original_line_time(iterator ll);
276276

277-
Result<shared_buffer_ref, std::string> read_line(iterator ll);
277+
Result<shared_buffer_ref, std::string> read_line(iterator ll,
278+
subline_options opts = {});
278279

279280
enum class read_format_t {
280281
plain,

‎src/logfile_sub_source.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2623,7 +2623,8 @@ logline_window::logmsg_info::get_metadata() const
26232623
Result<auto_buffer, std::string>
26242624
logline_window::logmsg_info::get_line_hash() const
26252625
{
2626-
auto sbr = TRY(this->li_file->read_line(this->li_logline));
2626+
auto fr = this->li_file->get_file_range(this->li_logline, false);
2627+
auto sbr = TRY(this->li_file->read_range(fr));
26272628
auto outbuf = auto_buffer::alloc(3 + hasher::STRING_SIZE);
26282629
outbuf.push_back('v');
26292630
outbuf.push_back('1');

‎src/session_data.cc‎

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,8 @@ bind_line(sqlite3* db,
170170
sqlite3_clear_bindings(stmt);
171171

172172
auto line_iter = lf->begin() + cl;
173-
auto read_result = lf->read_line(line_iter);
173+
auto fr = lf->get_file_range(line_iter, false);
174+
auto read_result = lf->read_range(fr);
174175

175176
if (read_result.isErr()) {
176177
return false;
@@ -556,8 +557,8 @@ load_time_bookmarks()
556557
= (const char*) sqlite3_column_text(stmt.in(), 7);
557558
const auto annotations = sqlite3_column_text(stmt.in(), 8);
558559
const auto log_opid = sqlite3_column_text(stmt.in(), 9);
559-
struct timeval log_tv;
560-
struct exttm log_tm;
560+
timeval log_tv;
561+
exttm log_tm;
561562

562563
if (last_mark_time == -1) {
563564
last_mark_time = mark_time;
@@ -601,7 +602,8 @@ load_time_bookmarks()
601602

602603
auto cl = content_line_t(
603604
std::distance(lf->begin(), line_iter));
604-
auto read_result = lf->read_line(line_iter);
605+
auto fr = lf->get_file_range(line_iter, false);
606+
auto read_result = lf->read_range(fr);
605607

606608
if (read_result.isErr()) {
607609
break;
@@ -616,8 +618,29 @@ load_time_bookmarks()
616618
.to_string();
617619

618620
if (line_hash != log_hash) {
619-
++line_iter;
620-
continue;
621+
// Using the formatted line for JSON-lines logs was
622+
// a mistake in earlier versions. To carry forward
623+
// older bookmarks, we need to replicate the bad
624+
// behavior.
625+
auto hack_read_res
626+
= lf->read_line(line_iter, {false, true});
627+
if (hack_read_res.isErr()) {
628+
break;
629+
}
630+
auto hack_sbr = hack_read_res.unwrap();
631+
auto hack_hash = hasher()
632+
.update(hack_sbr.get_data(),
633+
hack_sbr.length())
634+
.update(cl)
635+
.to_string();
636+
if (hack_hash == log_hash) {
637+
log_trace("needed hack to match line: %s:%d",
638+
lf->get_filename_as_string().c_str(),
639+
cl);
640+
} else {
641+
++line_iter;
642+
continue;
643+
}
621644
}
622645
auto& bm_meta = lf->get_bookmark_metadata();
623646
auto line_number = static_cast<uint32_t>(
@@ -1037,7 +1060,8 @@ save_user_bookmarks(sqlite3* db,
10371060
sqlite3_clear_bindings(stmt);
10381061

10391062
const auto line_iter = lf->begin() + cl;
1040-
auto read_result = lf->read_line(line_iter);
1063+
auto fr = lf->get_file_range(line_iter, false);
1064+
auto read_result = lf->read_range(fr);
10411065

10421066
if (read_result.isErr()) {
10431067
continue;
@@ -1089,7 +1113,8 @@ save_meta_bookmarks(sqlite3* db, sqlite3_stmt* stmt, logfile* lf)
10891113
sqlite3_clear_bindings(stmt);
10901114

10911115
auto line_iter = lf->begin() + cl;
1092-
auto read_result = lf->read_line(line_iter);
1116+
auto fr = lf->get_file_range(line_iter, false);
1117+
auto read_result = lf->read_range(fr);
10931118

10941119
if (read_result.isErr()) {
10951120
continue;
@@ -1485,11 +1510,6 @@ save_time_bookmarks()
14851510

14861511
auto line_iter = lf->begin() + lf->get_time_offset_line();
14871512
auto offset = lf->get_time_offset();
1488-
auto read_result = lf->read_line(line_iter);
1489-
1490-
if (read_result.isErr()) {
1491-
return;
1492-
}
14931513

14941514
bind_values(stmt.in(),
14951515
lf->original_line_time(line_iter),

‎src/themes/night-owl.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@
185185
"bold": true
186186
},
187187
"comment": {
188-
"color": "#676e95"
188+
"color": "#5d9899"
189189
},
190190
"doc-directive": {
191191
"color": "#addb67"

0 commit comments

Comments
 (0)