Code review of the 1C-Bitrix project

Code Review for a 1C-Bitrix Project

When a project transfers to a new team, has accumulated years of custom code, or is preparing to scale — an independent assessment of code quality is needed. A Bitrix project code review is more complex than a standard review: it requires understanding both modern PHP practices and the specifics of the Bitrix core — how to work with components correctly, when to use the D7 API instead of the legacy core, and why direct queries to b_iblock_element via CIBlockElement::GetList with full iteration destroy the database.

What Is Checked During Review

Architecture and code organization:

• Presence of custom modules (/local/modules/) instead of modifications in /bitrix/modules/
• Use of namespaces and autoloading
• Separation of concerns: controllers contain no SQL, templates contain no business logic
• Custom tables implemented via \Bitrix\Main\ORM\Data\DataManager, not raw queries

Database usage:

Typical issues we look for:

// BAD: N+1, fetching all elements without a limit
while ($arItem = $rsItems->Fetch()) {
    $price = \CPrice::GetList([], ['PRODUCT_ID' => $arItem['ID']])->Fetch();
}

// GOOD: pre-fetching prices in bulk
$productIds = array_column($items, 'ID');
$prices = \Bitrix\Catalog\PriceTable::getList([
    'filter' => ['=PRODUCT_ID' => $productIds],
])->fetchAll();
$priceMap = array_column($prices, 'PRICE', 'PRODUCT_ID');

Security:

• SQL injection via query concatenation without escaping
• XSS — outputting $_GET/$_POST without htmlspecialchars()
• Direct file access without checking B_PROLOG_INCLUDED
• API keys stored in code instead of module settings

Cache usage:

• Components with heavy queries and no caching
• Incorrect cache tag management (invalidation does not fire on element change)
• No caching on catalog AJAX endpoints

D7 vs legacy core:

Review Process

1. Static analysis — run PHPStan (level 5–6) and PHP_CodeSniffer with PSR-12 rules. Automatically detects approximately 40% of issues.

2. Manual analysis — review key subsystems: catalog components, checkout flow, 1C integration, authorization modules.

3. Query profiling — enable define('BX_DEBUG', true) on a dev environment, check the number of SQL queries on catalog pages and product cards. Acceptable threshold: up to 30 queries. Over 100 is a systemic problem.

4. Report — a structured document classifying issues by severity (Critical / High / Medium / Low), with an example of the problematic code and a fix recommendation.

Case Study: Bitrix Marketplace, 5 Years of Development

Situation: the project was developed over 5 years by different teams; the current vendor reports that adding new functionality is impossible "due to poor architecture."

What was found during the review:

• 847 direct calls to $DB->Query() with concatenation and no escaping — 12 potential SQL injection points
• N+1 in the listing component: 1 query for the list + N queries for prices (at N=50 — 51 queries instead of 2)
• 3 locations storing passwords as unsalted MD5
• Business logic in component template.php files — impossible to reuse or test
• 40% of custom code in /bitrix/ — any core update is guaranteed to break the site

Review outcome: a prioritized 6-month refactoring plan organized by epics. Critical vulnerabilities were resolved within 2 weeks.

What Is Included in Code Review

• Static analysis with PHPStan + PHPCS and a violations report
• Manual review of key subsystems: catalog, orders, integrations, authorization
• SQL query profiling on a dev environment
• Security audit: SQL injection, XSS, secrets storage
• Final report with severity classification and recommendations
• Refactoring plan consultation