Skip to content

Feature/http stream #1607 #1608

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
asosnoviy wants to merge 2 commits into from
Closed

Feature/http stream #1607 #1608

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e17d235
Переименован ВызватьHTTPМетод
asosnoviy Nov 12, 2025
d433762
Добавлен метод ВызватьHTTPМетодПоток
asosnoviy Nov 12, 2025
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
20 changes: 16 additions & 4 deletions src/OneScript.StandardLibrary/Http/HttpConnectionContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*----------------------------------------------------------
/*----------------------------------------------------------
This Source Code Form is subject to the terms of the
Mozilla Public License, v.2.0. If a copy of the MPL
was not distributed with this file, You can obtain one
Expand Down Expand Up @@ -199,11 +199,23 @@ public HttpResponseContext Head(HttpRequestContext request)
/// <param name="output">Строка. Имя выходного файла</param>
/// <returns>HTTPОтвет. Ответ сервера.</returns>
[ContextMethod("ВызватьHTTPМетод", "CallHTTPMethod")]
public HttpResponseContext Patch(string method, HttpRequestContext request, string output = null)
public HttpResponseContext CallHTTPMethod(string method, HttpRequestContext request, string output = null)
{
return GetResponse(request, method, output);
}

/// <summary>
/// Вызвать произвольный HTTP-метод с потоком ответа
/// </summary>
/// <param name="method">Строка. Имя метода HTTP</param>
/// <param name="request">HTTPЗапрос. Данные и заголовки запроса http</param>
/// <returns>HTTPОтвет. Ответ сервера.</returns>
[ContextMethod("ВызватьHTTPМетодПоток", "CallHTTPMethodStream")]
public HttpResponseContext CallHTTPMethodStream(string method, HttpRequestContext request)
{
return GetResponse(request, method, null, true);
}
Comment on lines +207 to +217
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 12, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify HttpWebResponse disposal in streaming scenarios.

The streaming implementation looks correct and follows appropriate patterns. The method properly delegates to GetResponse with rawStream=true and hardcodes output to null (which is appropriate for streaming).

However, please verify that HttpWebResponse resources are properly managed when streaming. In the current implementation:

  • The response object (line 364) is passed to HttpResponseContext but not explicitly stored or disposed
  • The stream keeps the response alive, but if callers don't properly dispose the stream returned by GetBodyAsStream(), connections may leak
  • Streaming scenarios can hold connections longer, making resource leaks more impactful

Consider verifying:

  1. That GenericStream properly disposes the underlying stream (the true parameter at HttpResponseContext line 164 suggests it does)
  2. Whether HttpResponseContext should hold a reference to the response and implement IDisposable to ensure cleanup
  3. Adding usage documentation about the importance of disposing the stream

🏁 Script executed:

#!/bin/bash
# Verify GenericStream disposal behavior and IDisposable implementation

# Check if GenericStream properly implements disposal
ast-grep --pattern 'class GenericStream $$$'

# Check if HttpResponseContext implements IDisposable
rg -n "class HttpResponseContext" -A 5

Length of output: 105523


---

🏁 Script executed:

```shell
#!/bin/bash
# Check HttpResponseContext Dispose implementation and GetResponse method
echo "=== HttpResponseContext Dispose Implementation ==="
grep -n "Dispose\|GetResponse" src/OneScript.StandardLibrary/Http/HttpResponseContext.cs | head -20

echo ""
echo "=== Looking for GetResponse method in HttpConnectionContext ==="
grep -n -A 15 "GetResponse" src/OneScript.StandardLibrary/Http/HttpConnectionContext.cs | head -40

Length of output: 2006


---

🏁 Script executed:

```shell
#!/bin/bash
# Find the private GetResponse method implementation
echo "=== GetResponse private method in HttpConnectionContext ==="
grep -n -A 25 "private.*GetResponse" src/OneScript.StandardLibrary/Http/HttpConnectionContext.cs | head -50

echo ""
echo "=== HttpResponseBody class definition and Dispose ==="
grep -n -B 2 -A 15 "class HttpResponseBody" src/OneScript.StandardLibrary/Http/HttpResponseBody.cs

Length of output: 1861


---

🏁 Script executed:

```shell
#!/bin/bash
# Find HttpResponseContext constructor
echo "=== HttpResponseContext Constructor ==="
grep -n -A 20 "public HttpResponseContext" src/OneScript.StandardLibrary/Http/HttpResponseContext.cs | head -30

echo ""
echo "=== HttpResponseBody full Dispose implementation ==="
grep -n -A 25 "public void Dispose" src/OneScript.StandardLibrary/Http/HttpResponseBody.cs

echo ""
echo "=== Check if response is stored in HttpResponseContext ==="
grep -n "HttpWebResponse\|_response" src/OneScript.StandardLibrary/Http/HttpResponseContext.cs

Length of output: 1327


---

🏁 Script executed:

```shell
#!/bin/bash
# Check _rawStream field and complete Dispose implementation
echo "=== HttpResponseContext fields and full Dispose ==="
grep -n "_rawStream\|private\|public void Dispose" src/OneScript.StandardLibrary/Http/HttpResponseContext.cs | head -20

echo ""
echo "=== Full HttpResponseContext.Dispose and related code ==="
sed -n '190,210p' src/OneScript.StandardLibrary/Http/HttpResponseContext.cs

echo ""
echo "=== HttpResponseBody Dispose(bool) implementation ==="
sed -n '208,232p' src/OneScript.StandardLibrary/Http/HttpResponseBody.cs

Length of output: 1530

Store and dispose HttpWebResponse in streaming scenarios to prevent connection leaks.

The streaming implementation has a critical resource leak. In the CallHTTPMethodStream code path:

  • The HttpWebResponse object created at line 364 in GetResponse() is passed to HttpResponseContext but never stored or disposed
  • HttpResponseContext.Dispose() only disposes _body, ignoring both _rawStream and the response object
  • In contrast, the non-streaming path correctly uses using(response) to dispose the response
  • Unreferenced HttpWebResponse objects leak, exhausting the connection pool—particularly problematic in streaming scenarios where connections are held longer

Fix: Store the HttpWebResponse as a private field in HttpResponseContext and dispose it in the Dispose() method, similar to how the non-streaming path handles it with using().


private HttpWebRequest CreateRequest(string resource)
{
var uriBuilder = new UriBuilder(_hostUri);
Expand Down Expand Up @@ -335,7 +347,7 @@ private static List<Range> ParseRange(string rangeHeader)
return range;
}

private HttpResponseContext GetResponse(HttpRequestContext request, string method, string output = null)
private HttpResponseContext GetResponse(HttpRequestContext request, string method, string output = null, bool rawStream = false)
Copy link
Owner

@EvilBeaver EvilBeaver Nov 12, 2025

Choose a reason for hiding this comment

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

А если просто без дополнительных параметров и методов - отдавать сырой поток в методе ПолучитьТелоКакПоток? так не получается?

Copy link
Contributor Author

@asosnoviy asosnoviy Nov 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Я не разобрался зачем он вообще вчитывается в методах заполнения тела =(

Copy link
Owner

@EvilBeaver EvilBeaver Nov 13, 2025

Choose a reason for hiding this comment

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

Может попробовать сделать все-таки? Мне кажется, там не обязательно трогать сетевой поток, если не вызывались методы ПолучитьТелоКакСтроку/ДвоичныеДанные

Copy link
Contributor Author

@asosnoviy asosnoviy Nov 13, 2025

Choose a reason for hiding this comment

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

Попробовать могу, но точно что то отломиться т.к. сейчас поток для чего то вычитывается в боди...

{
var webRequest = CreateRequest(request.ResourceAddress);
webRequest.AllowAutoRedirect = AllowAutoRedirect;
Expand All @@ -359,7 +371,7 @@ private HttpResponseContext GetResponse(HttpRequestContext request, string metho
throw;
}

var responseContext = new HttpResponseContext(response, output);
var responseContext = new HttpResponseContext(response, output, rawStream);

return responseContext;

Expand Down
28 changes: 20 additions & 8 deletions src/OneScript.StandardLibrary/Http/HttpResponseContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*----------------------------------------------------------
/*----------------------------------------------------------
This Source Code Form is subject to the terms of the
Mozilla Public License, v.2.0. If a copy of the MPL
was not distributed with this file, You can obtain one
Expand Down Expand Up @@ -28,18 +28,27 @@ public class HttpResponseContext : AutoContext<HttpResponseContext>, IDisposable
// TODO: Нельзя выделить массив размером больше чем 2GB
// поэтому функционал сохранения в файл не должен использовать промежуточный буфер _body
private HttpResponseBody _body;

private Stream _rawStream;

private string _defaultCharset;
private string _filename;

public HttpResponseContext(HttpWebResponse response)
public HttpResponseContext(HttpWebResponse response, string dumpToFile, bool rawStream)
{
RetrieveResponseData(response, null);
}

public HttpResponseContext(HttpWebResponse response, string dumpToFile)
{
RetrieveResponseData(response, dumpToFile);
if (!rawStream)
{

RetrieveResponseData(response, dumpToFile);
}
else
{
StatusCode = (int)response.StatusCode;
_defaultCharset = response.CharacterSet;

ProcessHeaders(response.Headers);
_rawStream = response.GetResponseStream();
}
}
Comment on lines +36 to 52
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 12, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

КРИТИЧЕСКАЯ ПРОБЛЕМА: Утечка ресурсов HTTP-ответа.

В режиме rawStream=true объект response не освобождается. В отличие от RetrieveResponseData (строка 56), который использует using(response), путь обработки сырого потока (строки 44-51) не вызывает Dispose() для объекта HttpWebResponse. Это приведет к утечке соединений, памяти и дескрипторов.

Примените этот diff для сохранения ссылки на response и его освобождения:

+        private HttpWebResponse _response;
+
         public HttpResponseContext(HttpWebResponse response, string dumpToFile, bool rawStream)
         {

             if (!rawStream)
             {

                 RetrieveResponseData(response, dumpToFile);
             }
             else
             {
                 StatusCode = (int)response.StatusCode;
                 _defaultCharset = response.CharacterSet;

                 ProcessHeaders(response.Headers);
                 _rawStream = response.GetResponseStream();
+                _response = response;
             }
         }

Затем обновите метод Dispose():

         public void Dispose()
         {
+            if (_response != null)
+            {
+                _response.Dispose();
+                _response = null;
+            }
+            
             if (_body != null)
+            {
                 _body.Dispose();
+                _body = null;
+            }
         }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In src/OneScript.StandardLibrary/Http/HttpResponseContext.cs around lines 36 to
52, the rawStream branch stores the response stream but never retains or
disposes the HttpWebResponse causing resource leaks; modify the constructor to
save the passed HttpWebResponse to a private field (e.g. _response) when
rawStream is true instead of only calling GetResponseStream(), and ensure
ProcessHeaders/StatusCode/_defaultCharset remain set; then update the class
Dispose() to check and call Dispose() (or Close()) on the stored _response (and
null it) in addition to disposing the response stream so the HttpWebResponse is
always released.


private void RetrieveResponseData(HttpWebResponse response, string dumpToFile)
Expand Down Expand Up @@ -151,6 +160,9 @@ public IValue GetBodyAsBinaryData()
[ContextMethod("ПолучитьТелоКакПоток", "GetBodyAsStream")]
public IValue GetBodyAsStream()
{
if(_rawStream != null)
return new GenericStream(_rawStream, true);

if (_body == null)
return ValueFactory.Create();

Expand Down