fix(baileys): enforce correct SOCKS5 protocol usage in service integr…#2416
Open
wikirp wants to merge 1 commit intoEvolutionAPI:mainfrom
Open
fix(baileys): enforce correct SOCKS5 protocol usage in service integr…#2416wikirp wants to merge 1 commit intoEvolutionAPI:mainfrom
wikirp wants to merge 1 commit intoEvolutionAPI:mainfrom
Conversation
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts proxy URL handling in the Baileys WhatsApp integration so that the configured protocol (including SOCKS5) is preserved and properly applied when building dynamic proxy URLs, instead of always forcing HTTP, and avoids empty entries from the proxy source list. Sequence diagram for proxy URL resolution with SOCKS5 supportsequenceDiagram
actor Admin
participant BaileysStartupService
participant Axios
participant ProxySource
participant ProxyAgentFactory
Admin->>BaileysStartupService: startWhatsAppInstance()
BaileysStartupService->>Axios: get(localProxy.host)
Axios->>ProxySource: HTTP GET proxy list
ProxySource-->>Axios: text list host:port per line
Axios-->>BaileysStartupService: response.data
BaileysStartupService->>BaileysStartupService: proxyUrls = text.split("\r\n").filter(Boolean)
BaileysStartupService->>BaileysStartupService: rand = randomIndex(proxyUrls.length)
BaileysStartupService->>BaileysStartupService: proxyUrl = proxyUrls[rand]
alt proxyUrl has protocol
BaileysStartupService->>BaileysStartupService: use proxyUrl as is
else proxyUrl without protocol
BaileysStartupService->>BaileysStartupService: proto = localProxy.protocol or http
BaileysStartupService->>BaileysStartupService: proxyUrl = proto + "://" + proxyUrl
end
BaileysStartupService->>ProxyAgentFactory: makeProxyAgent(proxyUrl)
ProxyAgentFactory-->>BaileysStartupService: agent
BaileysStartupService->>ProxyAgentFactory: makeProxyAgentUndici(proxyUrl)
ProxyAgentFactory-->>BaileysStartupService: fetchAgent
note over BaileysStartupService,ProxyAgentFactory: If proto is socks5, SOCKS5 handshake is used instead of HTTP CONNECT
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The new proxy selection logic still assumes
proxyUrls.length > 0; it might be safer to explicitly handle the case where no valid lines remain afterfilter(Boolean)(e.g., log and disable the proxy or fall back cleanly) instead of relying onMath.random()with length 0. - The inline comment added above the protocol handling is in Spanish; consider converting it to English to stay consistent with the rest of the codebase and make it easier for all contributors to understand.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new proxy selection logic still assumes `proxyUrls.length > 0`; it might be safer to explicitly handle the case where no valid lines remain after `filter(Boolean)` (e.g., log and disable the proxy or fall back cleanly) instead of relying on `Math.random()` with length 0.
- The inline comment added above the protocol handling is in Spanish; consider converting it to English to stay consistent with the rest of the codebase and make it easier for all contributors to understand.
## Individual Comments
### Comment 1
<location> `src/api/integrations/channel/whatsapp/whatsapp.baileys.service.ts:610-611` </location>
<code_context>
const response = await axios.get(this.localProxy?.host);
const text = response.data;
- const proxyUrls = text.split('\r\n');
+ const proxyUrls = text.split('\r\n').filter(Boolean);
const rand = Math.floor(Math.random() * Math.floor(proxyUrls.length));
- const proxyUrl = 'http://' + proxyUrls[rand];
</code_context>
<issue_to_address>
**suggestion:** Use a more robust line-splitting strategy for different newline conventions.
Splitting only on `"\r\n"` assumes Windows line endings. If the list is served with Unix-style `"\n"` only, you’ll get a single, newline-containing entry and `filter(Boolean)` won’t fix it. Using something like `text.split(/\r?\n/).filter(Boolean)` will correctly handle both CRLF and LF cases.
```suggestion
const text = response.data;
const proxyUrls = text.split(/\r?\n/).filter(Boolean);
```
</issue_to_address>
### Comment 2
<location> `src/api/integrations/channel/whatsapp/whatsapp.baileys.service.ts:612-613` </location>
<code_context>
const text = response.data;
- const proxyUrls = text.split('\r\n');
+ const proxyUrls = text.split('\r\n').filter(Boolean);
const rand = Math.floor(Math.random() * Math.floor(proxyUrls.length));
- const proxyUrl = 'http://' + proxyUrls[rand];
+ let proxyUrl = proxyUrls[rand];
+ // Si la línea ya tiene protocolo, úsala tal cual. Si no, anteponer el protocolo configurado
+ if (!/^\w+:\/\//.test(proxyUrl)) {
</code_context>
<issue_to_address>
**issue:** Handle the case where the filtered proxy list is empty before picking a random entry.
With `.filter(Boolean)`, `proxyUrls` can now be an empty array (e.g., blank/whitespace response). In that case `Math.random() * 0` produces `NaN`, so `proxyUrls[rand]` is `undefined` and gets passed to `makeProxyAgent`. Please add a guard for `proxyUrls.length === 0` and either skip proxy usage or log and fall back before computing `rand`.
</issue_to_address>
### Comment 3
<location> `src/api/integrations/channel/whatsapp/whatsapp.baileys.service.ts:613-618` </location>
<code_context>
+ let proxyUrl = proxyUrls[rand];
+ // Si la línea ya tiene protocolo, úsala tal cual. Si no, anteponer el protocolo configurado
+ if (!/^\w+:\/\//.test(proxyUrl)) {
+ const proto = this.localProxy?.protocol?.replace(':', '') || 'http';
+ proxyUrl = `${proto}://${proxyUrl}`;
+ }
options = { agent: makeProxyAgent(proxyUrl), fetchAgent: makeProxyAgentUndici(proxyUrl) };
</code_context>
<issue_to_address>
**suggestion:** Protocol normalization may produce malformed URLs if the configured protocol includes `://`.
If `this.localProxy.protocol` is set to values like `'http://'` or `'http://:'`, using `.replace(':', '')` leaves `proto` as `'http//'`, yielding URLs like `'http///host:port'`. Consider normalizing with something like `protocol.replace(/:.*$/, '')`, stripping `://` explicitly, or enforcing a punctuation-free protocol value at config load time.
```suggestion
let proxyUrl = proxyUrls[rand];
// Si la línea ya tiene protocolo, úsala tal cual. Si no, anteponer el protocolo configurado
if (!/^\w+:\/\//.test(proxyUrl)) {
const rawProtocol = this.localProxy?.protocol?.trim();
const proto = rawProtocol ? rawProtocol.replace(/:.*$/, '') : 'http';
proxyUrl = `${proto}://${proxyUrl}`;
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
610
to
+611
| const text = response.data; | ||
| const proxyUrls = text.split('\r\n'); | ||
| const proxyUrls = text.split('\r\n').filter(Boolean); |
Contributor
There was a problem hiding this comment.
suggestion: Use a more robust line-splitting strategy for different newline conventions.
Splitting only on "\r\n" assumes Windows line endings. If the list is served with Unix-style "\n" only, you’ll get a single, newline-containing entry and filter(Boolean) won’t fix it. Using something like text.split(/\r?\n/).filter(Boolean) will correctly handle both CRLF and LF cases.
Suggested change
| const text = response.data; | |
| const proxyUrls = text.split('\r\n'); | |
| const proxyUrls = text.split('\r\n').filter(Boolean); | |
| const text = response.data; | |
| const proxyUrls = text.split(/\r?\n/).filter(Boolean); |
Comment on lines
612
to
+613
| const rand = Math.floor(Math.random() * Math.floor(proxyUrls.length)); | ||
| const proxyUrl = 'http://' + proxyUrls[rand]; | ||
| let proxyUrl = proxyUrls[rand]; |
Contributor
There was a problem hiding this comment.
issue: Handle the case where the filtered proxy list is empty before picking a random entry.
With .filter(Boolean), proxyUrls can now be an empty array (e.g., blank/whitespace response). In that case Math.random() * 0 produces NaN, so proxyUrls[rand] is undefined and gets passed to makeProxyAgent. Please add a guard for proxyUrls.length === 0 and either skip proxy usage or log and fall back before computing rand.
Comment on lines
+613
to
+618
| let proxyUrl = proxyUrls[rand]; | ||
| // Si la línea ya tiene protocolo, úsala tal cual. Si no, anteponer el protocolo configurado | ||
| if (!/^\w+:\/\//.test(proxyUrl)) { | ||
| const proto = this.localProxy?.protocol?.replace(':', '') || 'http'; | ||
| proxyUrl = `${proto}://${proxyUrl}`; | ||
| } |
Contributor
There was a problem hiding this comment.
suggestion: Protocol normalization may produce malformed URLs if the configured protocol includes ://.
If this.localProxy.protocol is set to values like 'http://' or 'http://:', using .replace(':', '') leaves proto as 'http//', yielding URLs like 'http///host:port'. Consider normalizing with something like protocol.replace(/:.*$/, ''), stripping :// explicitly, or enforcing a punctuation-free protocol value at config load time.
Suggested change
| let proxyUrl = proxyUrls[rand]; | |
| // Si la línea ya tiene protocolo, úsala tal cual. Si no, anteponer el protocolo configurado | |
| if (!/^\w+:\/\//.test(proxyUrl)) { | |
| const proto = this.localProxy?.protocol?.replace(':', '') || 'http'; | |
| proxyUrl = `${proto}://${proxyUrl}`; | |
| } | |
| let proxyUrl = proxyUrls[rand]; | |
| // Si la línea ya tiene protocolo, úsala tal cual. Si no, anteponer el protocolo configurado | |
| if (!/^\w+:\/\//.test(proxyUrl)) { | |
| const rawProtocol = this.localProxy?.protocol?.trim(); | |
| const proto = rawProtocol ? rawProtocol.replace(/:.*$/, '') : 'http'; | |
| proxyUrl = `${proto}://${proxyUrl}`; | |
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📋 Description
This PR fixes a critical issue in whatsapp.baileys.service.ts regarding proxy configuration. It now verifies if a protocol is already defined in the proxy settings and respects it instead of defaulting to http.
Previously, the logic incorrectly forced an HTTP fallback even when socks5 was explicitly configured. This behavior caused connection failures with strict SOCKS5 proxies (such as Rust or Go-based implementations) because the client inadvertently sent HTTP CONNECT headers (0x67) instead of the required SOCKS handshake (0x05).
The fix enforces the use of socksDispatcher when a SOCKS proxy is detected or explicitly configured, ensuring the correct handshake protocol is established.
🔗 Related Issue
Closes #
🧪 Type of Change
🐛 Bug fix (non-breaking change which fixes an issue)
✨ New feature (non-breaking change which adds functionality)
💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
📚 Documentation update
🔧 Refactoring (no functional changes)
⚡ Performance improvement
🧹 Code cleanup
🔒 Security fix
🧪 Testing
Manual testing completed
Functionality verified in development environment
No breaking changes introduced
Tested with different connection types (Verified with strict SOCKS5 Residential Proxy)
Test Scenario:
Configured a WhatsApp instance using a strict SOCKS5 proxy (Rust implementation) without HTTP support.
Before fix: Connection failed immediately with "Protocol Incorrect" errors on the proxy server (Client sent CONNECT verb).
After fix: The client correctly initiated the SOCKS5 handshake, the QR code loaded successfully, and the instance reached "Connected" status.
📸 Screenshots (if applicable)
(You can drag and drop your "Connected" screenshot here to show proof of success)
✅ Checklist
My code follows the project's style guidelines
I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have manually tested my changes thoroughly
I have verified the changes work with different scenarios
Any dependent changes have been merged and published
📝 Additional Notes
This change is essential for users deploying Evolution API behind residential proxies or private tunneling infrastructures that enforce strict protocol compliance and do not support HTTP tunneling on SOCKS ports.
Summary by Sourcery
Bug Fixes: