Skip to content
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.

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

In ConvertFrom-Json, the max depth for deserialization is 1024, however, we should investigate if the current implementation can handle object of greater depth. #3182

Closed
Francisco-Gamino opened this issue Feb 22, 2017 · 10 comments

Comments

@Francisco-Gamino
Copy link
Contributor

@Francisco-Gamino Francisco-Gamino commented Feb 22, 2017

Steps to reproduce

# Create nested JSON data with max depth of 1025
$maxDepth = 1025
$val = ""
$end="null"

1..$maxDepth | % {
    $val += '{"' + $_+'":'
    $end += '}'
}
$val + $end  | ConvertFrom-Json 

Expected behavior

PowerShell is able to deserialize objects with depth greater of 1024.

Actual behavior

ConvertFrom-Json : Conversion from JSON failed with error: The reader's MaxDepth of 1024 has been exceeded. Path '1.2.3.4.5.6.7.8.9.10.11.12.13.14.15.16.17.18.19.20.21.22.23.24.25.26.27.28.29.30.31.32.33.34.35.36.37.38.39.40.41.42.43.44
.45.46.47.48.49.50.51.52.53.54.55.56.57.58.59.60.61.62.63.64.65.66.67.68.69.70.71.72.73.74.75.76.77.78.79.80.81.82.83.84.85.86.87.88.89.90.91.92.93.94.95.96.97.98.99.100.101.102.103.104.105.106.107.108.109.110.111.112.113.114.115.116.11
7.118.119.120.121.122.123.124.125.126.127.128.129.130.131.132.133.134.135.136.137.138.139.140.141.142.143.144.145.146.147.148.149.150.151.152.153.154.155.156.157.158.159.160.161.162.163.164.165.166.167.168.169.170.171.172.173.174.175.17
6.177.178.179.180.181.182.183.184.185.186.187.188.189.190.191.192.193.194.195.196.197.198.199.200.201.202.203.204.205.206.207.208.209.210.211.212.213.214.215.216.217.218.219.220.221.222.223.224.225.226.227.228.229.230.231.232.233.234.23
5.236.237.238.239.240.241.242.243.244.245.246.247.248.249.250.251.252.253.254.255.256.257.258.259.260.261.262.263.264.265.266.267.268.269.270.271.272.273.274.275.276.277.278.279.280.281.282.283.284.285.286.287.288.289.290.291.292.293.29
4.295.296.297.298.299.300.301.302.303.304.305.306.307.308.309.310.311.312.313.314.315.316.317.318.319.320.321.322.323.324.325.326.327.328.329.330.331.332.333.334.335.336.337.338.339.340.341.342.343.344.345.346.347.348.349.350.351.352.35
3.354.355.356.357.358.359.360.361.362.363.364.365.366.367.368.369.370.371.372.373.374.375.376.377.378.379.380.381.382.383.384.385.386.387.388.389.390.391.392.393.394.395.396.397.398.399.400.401.402.403.404.405.406.407.408.409.410.411.41
2.413.414.415.416.417.418.419.420.421.422.423.424.425.426.427.428.429.430.431.432.433.434.435.436.437.438.439.440.441.442.443.444.445.446.447.448.449.450.451.452.453.454.455.456.457.458.459.460.461.462.463.464.465.466.467.468.469.470.47
1.472.473.474.475.476.477.478.479.480.481.482.483.484.485.486.487.488.489.490.491.492.493.494.495.496.497.498.499.500.501.502.503.504.505.506.507.508.509.510.511.512.513.514.515.516.517.518.519.520.521.522.523.524.525.526.527.528.529.53
0.531.532.533.534.535.536.537.538.539.540.541.542.543.544.545.546.547.548.549.550.551.552.553.554.555.556.557.558.559.560.561.562.563.564.565.566.567.568.569.570.571.572.573.574.575.576.577.578.579.580.581.582.583.584.585.586.587.588.58
9.590.591.592.593.594.595.596.597.598.599.600.601.602.603.604.605.606.607.608.609.610.611.612.613.614.615.616.617.618.619.620.621.622.623.624.625.626.627.628.629.630.631.632.633.634.635.636.637.638.639.640.641.642.643.644.645.646.647.64
8.649.650.651.652.653.654.655.656.657.658.659.660.661.662.663.664.665.666.667.668.669.670.671.672.673.674.675.676.677.678.679.680.681.682.683.684.685.686.687.688.689.690.691.692.693.694.695.696.697.698.699.700.701.702.703.704.705.706.70
7.708.709.710.711.712.713.714.715.716.717.718.719.720.721.722.723.724.725.726.727.728.729.730.731.732.733.734.735.736.737.738.739.740.741.742.743.744.745.746.747.748.749.750.751.752.753.754.755.756.757.758.759.760.761.762.763.764.765.76
6.767.768.769.770.771.772.773.774.775.776.777.778.779.780.781.782.783.784.785.786.787.788.789.790.791.792.793.794.795.796.797.798.799.800.801.802.803.804.805.806.807.808.809.810.811.812.813.814.815.816.817.818.819.820.821.822.823.824.82
5.826.827.828.829.830.831.832.833.834.835.836.837.838.839.840.841.842.843.844.845.846.847.848.849.850.851.852.853.854.855.856.857.858.859.860.861.862.863.864.865.866.867.868.869.870.871.872.873.874.875.876.877.878.879.880.881.882.883.88
4.885.886.887.888.889.890.891.892.893.894.895.896.897.898.899.900.901.902.903.904.905.906.907.908.909.910.911.912.913.914.915.916.917.918.919.920.921.922.923.924.925.926.927.928.929.930.931.932.933.934.935.936.937.938.939.940.941.942.94
3.944.945.946.947.948.949.950.951.952.953.954.955.956.957.958.959.960.961.962.963.964.965.966.967.968.969.970.971.972.973.974.975.976.977.978.979.980.981.982.983.984.985.986.987.988.989.990.991.992.993.994.995.996.997.998.999.1000.1001.
1002.1003.1004.1005.1006.1007.1008.1009.1010.1011.1012.1013.1014.1015.1016.1017.1018.1019.1020.1021.1022.1023.1024', line 1, position 7086.
At line:1 char:16
+ $val + $end  | ConvertFrom-Json
+                ~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [ConvertFrom-Json], ArgumentException
    + FullyQualifiedErrorId : System.ArgumentException,Microsoft.PowerShell.Commands.ConvertFromJsonCommand

Environment data

> $PSVersionTable

Name                           Value
----                           -----
WSManStackVersion              3.0
SerializationVersion           1.1.0.1
PSRemotingProtocolVersion      2.3
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
GitCommitId                    v6.0.0-alpha.13-139-g152de408523eef294ab8592dd7740dd86c93313f
CLRVersion
PSVersion                      6.0.0-alpha
PSEdition                      Core
BuildVersion                   3.0.0.0
@iSazonov

This comment has been minimized.

Copy link
Collaborator

@iSazonov iSazonov commented Aug 29, 2018

@markekraus Could you please look current status the Issue?

@louistio

This comment has been minimized.

Copy link
Contributor

@louistio louistio commented Sep 6, 2018

@iSazonov Considering this depth is manually set in JsonObject.cs:

new JsonSerializerSettings
{
    TypeNameHandling = TypeNameHandling.None,
    MetadataPropertyHandling = MetadataPropertyHandling.Ignore,
    MaxDepth = 1024
}

I think it would be very easy for the current implementation to handle objects of greater depth (by simply removing that setting, it is set to no maximum depth by default). However, I might be missing something here.

@iSazonov

This comment has been minimized.

Copy link
Collaborator

@iSazonov iSazonov commented Sep 6, 2018

@louistio Thanks! I think you found a right place. Do you want to offer a PR?

I don't know that new value should be - 2048? /cc @markekraus @Francisco-Gamino @SteveL-MSFT

@vexx32

This comment has been minimized.

Copy link
Collaborator

@vexx32 vexx32 commented Sep 7, 2018

Would it be insensible to have it at the largest possible value, and retain the default max depth setting according to the parameter of the cmdlet, which is by default only 2 (as I recall)?

@iSazonov

This comment has been minimized.

Copy link
Collaborator

@iSazonov iSazonov commented Sep 7, 2018

Good point @vexx32

@louistio

This comment has been minimized.

Copy link
Contributor

@louistio louistio commented Sep 7, 2018

@vexx32 Just to be clear, this issue is specifically talking about the ConvertFrom-Json cmdlet which throws an exception when the string contains a JSON object with a depth greater than 1024. I agree the cmdlet should not have a max depth, but what do you mean when you say:

and retain the default max depth setting according to the parameter of the cmdlet

Isn't that a parameter for the ConvertTo-Json cmdlet?

@SteveL-MSFT

This comment has been minimized.

Copy link
Member

@SteveL-MSFT SteveL-MSFT commented Sep 7, 2018

It seems ok to me to not set the default (which is null) as long as we add a parameter so the user can specify a maxdepth.

cc @TravisEz13

@vexx32

This comment has been minimized.

Copy link
Collaborator

@vexx32 vexx32 commented Sep 7, 2018

Oh, good point! I thought they both had a maxdepth param.

Then in that case, yes, the convertfrom should also have a maxdepth parameter. What a sensible default would be for that, I'm not sure, but even a small number would be fine, provided it's configurable.

@iSazonov

This comment has been minimized.

Copy link
Collaborator

@iSazonov iSazonov commented Sep 8, 2018

@louistio Do you want to start contribute in the repo? :-) Fixing the Issue is best for start.

@louistio

This comment has been minimized.

Copy link
Contributor

@louistio louistio commented Nov 7, 2018

@iSazonov Hey, sorry for taking so long, I got busy in the past weeks. I've opened up a PR over at #8199 according to what was discussed here. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

6 participants
You can’t perform that action at this time.