Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions src/pages/attendees/badge-checkin-page.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,17 @@
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
**/
* */

import React, { useState } from "react";
import { connect } from "react-redux";
import { Breadcrumb } from "react-breadcrumbs";
import T from "i18n-react/dist/i18n-react";
import { checkInBadge } from "../../actions/badge-actions";
import QrReader from "modern-react-qr-reader";
import { isMobile } from "react-device-detect";
import Swal from "sweetalert2";
import { checkInBadge } from "../../actions/badge-actions";
import { formatBadgeQR } from "../../utils/methods";
import styles from "../../styles/badge-checkin-page.module.less";

const BadgeCheckinPage = ({ match, currentSummit, checkInBadge }) => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

current summit is never used removed it if not used

Expand All @@ -27,12 +28,21 @@ const BadgeCheckinPage = ({ match, currentSummit, checkInBadge }) => {

const handleCheckIn = (data) => {
if (data && !scanning) {
const badgeData = formatBadgeQR(data, currentSummit);
if (!badgeData) {
Swal.fire(
T.translate("general.error"),
T.translate("badge_checkin.error_invalid_qr"),
"warning"
);
return;
}
setScanning(true);
checkInBadge(data)
.then(() => {
Swal.fire(
T.translate("badge_checkin.checked_in"),
`${qrValid[3]} (${qrValid[2]}) checked in!`,
`${badgeData.fullName} (${badgeData.email}) checked in!`,
"success"
);
})
Expand All @@ -44,8 +54,8 @@ const BadgeCheckinPage = ({ match, currentSummit, checkInBadge }) => {

const handleError = () => {
Swal.fire({
title: "Error",
text: "cannot read QR code, please try again",
title: T.translate("general.error"),
text: T.translate("badge_checkin.error_qr"),
type: "warning"
});
};
Expand Down Expand Up @@ -77,6 +87,7 @@ const BadgeCheckinPage = ({ match, currentSummit, checkInBadge }) => {
onClick={() =>
setCamera(camera === "environment" ? "user" : "environment")
}
type="button"
>
<i
className="fa fa-camera"
Expand Down
2 changes: 2 additions & 0 deletions src/utils/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,3 +224,5 @@ export const ROOM_OCCUPANCY_OPTIONS = [
"FULL",
"OVERFLOW"
];

export const BADGE_QR_MINIMUM_EXPECTED_FIELDS = 3;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, find and examine the constant definition
cat -n src/utils/constants.js | grep -A 2 -B 2 "BADGE_QR_MINIMUM_EXPECTED_FIELDS"

Repository: fntechgit/summit-admin

Length of output: 141


🏁 Script executed:

# Search for all usages of BADGE_QR_MINIMUM_EXPECTED_FIELDS
rg "BADGE_QR_MINIMUM_EXPECTED_FIELDS" -n -B 3 -A 3

Repository: fntechgit/summit-admin

Length of output: 969


🏁 Script executed:

# Find and examine the formatBadgeQR function
rg "formatBadgeQR" -n -A 20 src/utils/methods.js

Repository: fntechgit/summit-admin

Length of output: 560


Constant value doesn't align with array access pattern in formatBadgeQR.

The constant is set to 3, but formatBadgeQR in src/utils/methods.js accesses qrCodeArray[0], qrCodeArray[1], qrCodeArray[2], and qrCodeArray[3] (lines 517-520). The validation check at line 513 only guarantees qrCodeArray.length >= 3, which ensures indices 0-2 exist. Accessing qrCodeArray[3] without validating it first creates an inconsistency—the || "" fallback prevents crashes but doesn't properly validate the 4th field.

Change the constant to 4 to ensure all accessed array indices are validated before use.

-export const BADGE_QR_MINIMUM_EXPECTED_FIELDS = 3;
+export const BADGE_QR_MINIMUM_EXPECTED_FIELDS = 4;
🤖 Prompt for AI Agents
In @src/utils/constants.js around line 228, The constant
BADGE_QR_MINIMUM_EXPECTED_FIELDS is set to 3 but formatBadgeQR reads
qrCodeArray[0..3]; update BADGE_QR_MINIMUM_EXPECTED_FIELDS to 4 so the
validation (using BADGE_QR_MINIMUM_EXPECTED_FIELDS) guarantees indices 0 through
3 exist before formatBadgeQR accesses them, ensuring consistent validation for
the fourth field.

21 changes: 20 additions & 1 deletion src/utils/methods.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ import {
ONE_MINUTE,
INT_BASE,
OR_FILTER,
MARKETING_SETTING_TYPE_HEX_COLOR
MARKETING_SETTING_TYPE_HEX_COLOR,
BADGE_QR_MINIMUM_EXPECTED_FIELDS
} from "./constants";

const DAY_IN_SECONDS = 86400; // 86400 seconds per day
Expand Down Expand Up @@ -504,3 +505,21 @@ export const arrayToString = (array, join = ",") => {

export const capitalizeFirstLetter = (val) =>
String(val).charAt(0).toUpperCase() + String(val).slice(1);

export const formatBadgeQR = (code, summit) => {
const qrCodeArray = code.split(summit.qr_registry_field_delimiter);

if (
qrCodeArray.length >= BADGE_QR_MINIMUM_EXPECTED_FIELDS &&
qrCodeArray[0] === summit.badge_qr_prefix
) {
return {
badgePrefix: qrCodeArray[0],
ticketNumber: qrCodeArray[1],
email: qrCodeArray[2] || "",
fullName: qrCodeArray[3] || ""
};
}

return null;
};
Comment on lines +509 to +525
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add input validation and ensure array bounds are properly checked.

Several concerns with this function:

  1. Array access mismatch: The function checks qrCodeArray.length >= BADGE_QR_MINIMUM_EXPECTED_FIELDS (which is 3) but accesses qrCodeArray[3]. This doesn't align with the maintainer's previous feedback requesting a check for 4 elements.

  2. Missing null/undefined checks: The function doesn't validate that code, summit.qr_registry_field_delimiter, or summit.badge_qr_prefix are defined before use, which could cause runtime errors.

  3. Empty field validation: Consider validating that critical fields like ticketNumber and email are not empty strings after parsing, as empty values may cause issues downstream when displaying badge data in the SweetAlert popup.

Based on past review comments from smarcet.

🔎 Proposed improvements
 export const formatBadgeQR = (code, summit) => {
+  if (!code || !summit?.qr_registry_field_delimiter || !summit?.badge_qr_prefix) {
+    return null;
+  }
+
   const qrCodeArray = code.split(summit.qr_registry_field_delimiter);
 
   if (
     qrCodeArray.length >= BADGE_QR_MINIMUM_EXPECTED_FIELDS &&
     qrCodeArray[0] === summit.badge_qr_prefix
   ) {
+    const ticketNumber = qrCodeArray[1]?.trim();
+    const email = qrCodeArray[2]?.trim() || "";
+    const fullName = qrCodeArray[3]?.trim() || "";
+
+    // Validate that required fields are not empty
+    if (!ticketNumber) {
+      return null;
+    }
+
     return {
       badgePrefix: qrCodeArray[0],
-      ticketNumber: qrCodeArray[1],
-      email: qrCodeArray[2] || "",
-      fullName: qrCodeArray[3] || ""
+      ticketNumber,
+      email,
+      fullName
     };
   }
 
   return null;
 };

Note: Also ensure BADGE_QR_MINIMUM_EXPECTED_FIELDS is set to 4 as discussed in the constants.js review.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const formatBadgeQR = (code, summit) => {
const qrCodeArray = code.split(summit.qr_registry_field_delimiter);
if (
qrCodeArray.length >= BADGE_QR_MINIMUM_EXPECTED_FIELDS &&
qrCodeArray[0] === summit.badge_qr_prefix
) {
return {
badgePrefix: qrCodeArray[0],
ticketNumber: qrCodeArray[1],
email: qrCodeArray[2] || "",
fullName: qrCodeArray[3] || ""
};
}
return null;
};
export const formatBadgeQR = (code, summit) => {
if (!code || !summit?.qr_registry_field_delimiter || !summit?.badge_qr_prefix) {
return null;
}
const qrCodeArray = code.split(summit.qr_registry_field_delimiter);
if (
qrCodeArray.length >= BADGE_QR_MINIMUM_EXPECTED_FIELDS &&
qrCodeArray[0] === summit.badge_qr_prefix
) {
const ticketNumber = qrCodeArray[1]?.trim();
const email = qrCodeArray[2]?.trim() || "";
const fullName = qrCodeArray[3]?.trim() || "";
// Validate that required fields are not empty
if (!ticketNumber) {
return null;
}
return {
badgePrefix: qrCodeArray[0],
ticketNumber,
email,
fullName
};
}
return null;
};
🤖 Prompt for AI Agents
In @src/utils/methods.js around lines 509-525, formatBadgeQR currently assumes
inputs exist and accesses qrCodeArray[3] while only checking for
BADGE_QR_MINIMUM_EXPECTED_FIELDS (should be 4); update the function to first
validate that `code` and `summit` are defined and that
`summit.qr_registry_field_delimiter` and `summit.badge_qr_prefix` are non-empty
strings, then split using the delimiter and check that qrCodeArray.length >=
BADGE_QR_MINIMUM_EXPECTED_FIELDS (ensure BADGE_QR_MINIMUM_EXPECTED_FIELDS is set
to 4) before indexing; when building the return object, trim string fields and
validate that `ticketNumber` and `email` are non-empty (return null or a clear
error result if required) so no out-of-bounds or empty critical fields propagate
downstream.