Apologies if this shouldn't be a Security task.

In T187638#3981346, @JJMC89 wrote:

Apologies if this shouldn't be a Security task.

This definitely is a security task (leaking secret info)

Thank you for reporting this.

)

There's also an issue with Special:redirect/user if the user has been revdel'd

Code-Review-1

+			'log_type' => LogPage::DELETED_ACTION,

We were discussing this on T188145, I don't think this is correct.

+		// get rid of log_id and log_deleted.
+		array_shift( $logparams );
 		array_shift( $logparams );

This should probably just use unset().

+				if ( !LogEventsList::userCan(
+					$rowMain, $fieldToBitsMapping[$cond], $user
+				) ) {
+					// This field is redacted for main log entry,
+					// so treat as always matching.
+					$matchedRows[] = $row;
+					continue;
+				}

This could go outside the inner loop, just assign $matchedRows = $logsSameTimestamps and continue. If you change the code below to use $mainRow instead of $matchedRows[0], you could even skip the assignment.

Also, won't this and the bits below generate undefined index warnings when trying to access $fieldToBitsMapping[$cond] for $cond === 'log_timestamp'?

Thanks for the review. PS2:

-               array_shift( $logparams );
+               // get rid of log_id and log_deleted.
+               unset( $logparams[0] );
+               unset( $logparams[1] );

Huh? $logparams doesn't have a [0] or [1] ... Oh, I see, this needs a rebase on top of rMW27c61fb1e94d: Add `actor` table and code to start using it.

+                       if (
+                               $logKey === 'log_user_text' &&
+                               !LogEventsList::userCan(
+                                       $rowMain, LogPage::DELETED_USER, $user
+                               )
+                       ) {
+                               continue;
+                       }
                        $query[$keys[$logKey]] = $matchedRows[0]->$logKey;

I'd prefer it if you used either $rowMain or $matchedRows[0] consistently here.

Could this be fixed by removing the log look-up entirely from Special:Redirect and having a log-ID parameter to Special:Log? (Have done so in https://gerrit.wikimedia.org/r/#/c/428265/ for T191608).

that would certainly simplify the code

In T187638#4151786, @Samwilson wrote:

Could this be fixed by removing the log look-up entirely from Special:Redirect and having a log-ID parameter to Special:Log? (Have done so in https://gerrit.wikimedia.org/r/#/c/428265/ for T191608).

Do we want to backport this patch then?

Done in https://gerrit.wikimedia.org/r/#/q/I5f2e52531cd2ba617a25570e40aa8c5168e284d9

Does this patch fix the bug fully on it's own then?